-
Notifications
You must be signed in to change notification settings - Fork 3k
use script's baseurl for cssstylesheets #11706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
use script's baseurl for cssstylesheets #11706
Conversation
d9201c0
to
86fbf69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good if @nicolo-ribaudo reviewed this too. And maybe @dandclark.
source
Outdated
|
||
<li><p>Let <var>sheet</var> be the result of running the steps to <span>create a constructed | ||
<code>CSSStyleSheet</code></span> with an empty dictionary as the argument.</p></li> | ||
<code>CSSStyleSheet</code></span> with a dictionary with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<code>CSSStyleSheet</code></span> with a dictionary with: | |
<code>CSSStyleSheet</code></span> with |
(might have to line break differently after applying this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I see, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind changing the line wrapping but the entire «[ .* ]»
chunk will not fit on the 100 column break point on the line above, so it feels right to keep it on one line.
I see other examples do break on «[
, so I can put that on the line above, but it feels odd to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do break inside literals just like we break on whitespace elsewhere, but it might not matter much if we end up applying @zcorpan's rewrapper script.
<li><p>Set <var>script</var>'s <span data-x="concept-script-settings-object">settings | ||
object</span> to <var>settings</var>.</p></li> | ||
|
||
<li><p>Let <var>baseUrl</var> be <var>script</var>'s <span data-x="concept-script-base-url">base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure script
's baseURL
is valid here -- we just created script
in the first step of this algorithm and haven't initialized that field yet.
I see that other "create a module script" algos take the baseURL as a parameter, e.g. see create a javascript module script. It looks to me like "create a CSS module script" also needs that parameter. We'd pass in the "response's URL" as the baseURL here in the same way as is done for WASM module scripts and JS module scripts nearby.
It's a little more plumbing than I thought it'd be in #11629 (comment) but I hadn't realized at the time that the CSS module script's baseURL just gets set to null.
This aligns the spec to Chrome's implementation and WPTs found in web-platform-tests/wpt#29408, by setting the
baseUrl
of the CSSStyleSheet to be that of the script module's baseUrl.Refs #11629
/webappapis.html ( diff )