Fix outdated "destination is subresource" steps #1470
Conversation
source
Outdated
| <li><p>Let <var>request</var> be a new <span data-x="concept-request">request</span> whose | ||
| <span data-x="concept-request-url">url</span> is <var>url</var>, <span | ||
| data-x="concept-request-destination">destination</span> is "<code data-x="">subresource</code>", | ||
| data-x="concept-request-destination">destination</span> is "", |
There was a problem hiding this comment.
I prefer "the empty string" spelled out.
There was a problem hiding this comment.
Also, I think the various algorithms using "create a potential-CORS request" might have different destinations. So this might have to become an argument that is passed in. Did you check the various call sites?
There was a problem hiding this comment.
Also, I think the various algorithms using "create a potential-CORS request" might have different destinations. So this might have to become an argument that is passed in. Did you check the various call sites?
I did not yet, but I will.
|
There's also the 'destination is "unknown"' case we should change. Those need to become "embed" and "object" as far as I can tell. |
|
The issue this fixes is #731. |
Yes, just noticed those to. Will push another commit here to fix them. |
See b89cd82 |
source
Outdated
| <li><p>Let <var>request</var> be the result of <span | ||
| data-x="create a potential-CORS request">creating a potential-CORS request</span> given | ||
| <var>url</var> and <var>corsAttributeState</var>. | ||
| <var>url</var>, <var>destination</var> and <var>corsAttributeState</var>. |
There was a problem hiding this comment.
Inline "the empty string" here and add the Oxford comma.
|
Sorry for belated review, couldn't do much yesterday. |
Sorry for the belated response to the review—was working on landing an implementation of validator/validator#284 for the validator. But I believe now have all the updates pushed here for the responses to the review. |
source
Outdated
| data-x="create a potential-CORS request">creating a potential-CORS request</span> given | ||
| <var>url</var>, <var>destination</var> and <var>corsAttributeState</var>. | ||
| <var>url</var>, <var>destination</var> set to the empty string, and | ||
| <var>corsAttributeState</var>. |
There was a problem hiding this comment.
I think we should just have "the empty string" here. We generally don't do named arguments. Sorry for the ongoing nits.
There was a problem hiding this comment.
Same for the remainder. So "... given url, the empty string, and corsAttributeState."
This aligns “fetch a classic script” with the new “creating a potential-CORS request” signature.
No description provided.