General tidying, clearer adoptedStyleSheets, remove document restriction #60

rakina · 2018-10-24T14:51:45Z

Trying to resolve a lot of issues at once:

General tidying:

the asynchronous createCSSStyleSheet method isn't actually asynchronous #42 - Make waiting for imports asynchronous
Why have createEmptyCSSStyleSheet()? #43 - Remove createEmptyCSSStyleSheet, make text optional
spec should say where to file issues #44 - Add repo to metadata
introduction should explain why this is important for shadow DOM #47 - Added "motivation" section
empty section on applying styles in all contexts #51 - Removed unused sections
Why do we throw different exceptions for non-text/css imports and other load failures #53 - Make everything throw DOMExceptions instead
Do @imports inside @imports block the toplevel promise being resolved? #55 - Add line to wait for nested imports

Clearer adoptedStyleSheets

Make processing model more precise for adoptedStylesheets getter/setter #41, first paragraph about adopting constructed stylesheets should be clearer #50, Need some wordsmithing on the "using constructed stylesheets" section #52 - Added steps for adoptedStyleSheets, added associated document

Remove document restriction

Maybe we don't need documents after all #58 - Removed document restriction from spec (but there's still associated document, maybe instead we can use another way for marking constructed stylesheet?)

Since other open issues still need discussion, I'm only resolving issues mentioned above for now.

Preview | Diff

…ocument restriction

rakina · 2018-10-24T14:53:49Z

FYI @domenic and @bzbarsky.

domenic

Great start! Some cleanups possible.

domenic · 2018-10-26T10:31:12Z

index.bs

-	[NewObject] CSSStyleSheet createCSSStyleSheetSync(DOMString text, optional CSSStyleSheetInit options);
-	[NewObject] CSSStyleSheet createEmptyCSSStyleSheet(optional CSSStyleSheetInit options);
+	[NewObject] Promise&lt;CSSStyleSheet> createCSSStyleSheet(optional DOMString text, optional CSSStyleSheetInit options);
+	[NewObject] CSSStyleSheet createCSSStyleSheetSync(optional DOMString text, optional CSSStyleSheetInit options);


Probably want = "" on both text params

domenic · 2018-10-26T10:31:41Z

index.bs

-		7. Wait for loading of <a spec=css-cascade-4>@import</a> rules in <var>sheet</var>.
-			* If any of them resulted in a resource with a <a spec=html>Content-Type metadata</a> of anything other than <code>text/css</code>, reject <var>promise</var> with a {{TypeError}}.
-			* If any of them failed to load, reject <var>promise</var> with reason set to {{DOMException}} with the message "Loading import rules failed".
+		7. Wait for loading of <a spec=css-cascade-4>@import</a> rules in <var>sheet</var> and any nested loads from those rules in parallel.


It'll be more conventional if you put "In parallel," first, before "wait".

domenic · 2018-10-26T10:32:04Z

index.bs

-		7. Wait for loading of <a spec=css-cascade-4>@import</a> rules in <var>sheet</var>.
-			* If any of them resulted in a resource with a <a spec=html>Content-Type metadata</a> of anything other than <code>text/css</code>, reject <var>promise</var> with a {{TypeError}}.
-			* If any of them failed to load, reject <var>promise</var> with reason set to {{DOMException}} with the message "Loading import rules failed".
+		7. Wait for loading of <a spec=css-cascade-4>@import</a> rules in <var>sheet</var> and any nested loads from those rules in parallel.


"nested loads" is a bit confusing. "nested @imports" would be more precise.

domenic · 2018-10-26T10:34:00Z

index.bs

@@ -144,6 +128,17 @@ partial interface DocumentOrShadowRoot {
 	</dd>
 </dl>

-Applying Styles In All Contexts {#styles-in-all-contexts}
-===================
+Every {{DocumentOrShadowRoot}} has adopted stylesheets (of type FrozenArray&lt;CSSStyleSheet>).


Use <dfn> around "adopted stylesheets" here, then when referring to it later, say [=adopted stylesheets=] to link to it.

domenic · 2018-10-26T10:35:01Z

index.bs

-			* If any of them resulted in a resource with a <a spec=html>Content-Type metadata</a> of anything other than <code>text/css</code>, reject <var>promise</var> with a {{TypeError}}.
-			* If any of them failed to load, reject <var>promise</var> with reason set to {{DOMException}} with the message "Loading import rules failed".
+		7. Wait for loading of <a spec=css-cascade-4>@import</a> rules in <var>sheet</var> and any nested loads from those rules in parallel.
+			* If any of them failed to load or resulted in a resource with a <a spec=html>Content-Type metadata</a> of anything other than <code>text/css</code>, reject <var>promise</var> with reason set to {{DOMException}} with name {{NotAllowedError}}.


Here and elsewhere, you can use the more conventional "{{NotAllowedError}}" {{DOMException}}.

domenic · 2018-10-26T10:38:37Z

index.bs

@@ -144,6 +128,17 @@ partial interface DocumentOrShadowRoot {
 	</dd>
 </dl>


The dl here has a statement of fact, not a normative requirement. It's also at the wrong level, talking about the attribute instead of the "adopted stylesheets" concept. I would suggest deleting it, and instead putting the <dfn> around the getter.

domenic · 2018-10-26T10:40:51Z

index.bs

@@ -144,6 +128,17 @@ partial interface DocumentOrShadowRoot {
 	</dd>
 </dl>

-Applying Styles In All Contexts {#styles-in-all-contexts}
-===================
+Every {{DocumentOrShadowRoot}} has adopted stylesheets (of type FrozenArray&lt;CSSStyleSheet>).


Here is where you should add a normative requirement about how to treat the adopted stylesheets. For example

For documents, the user agent must include all style sheets in the {{Document}}'s [=adopted stylesheets=] inside its <a spec=cssom-1>document CSS style sheets</a> collection, ordered after all the other style sheets (i.e. those derived from {{Document/styleSheets}}).

Also, we need some statement for how this affects shadow roots?

I added this but combined it with shadow root also. I don't know where "shadow stylesheets" is defined though..

domenic · 2018-10-26T10:41:13Z

index.bs

+1. Let <var>adopted</var> be the result of converting the given value to a FrozenArray&lt;CSSStyleSheet>
+2. If any entry of <var>adopted</var> has no associated document (e.g. it's not made by factory methods to construct stylesheets), throw a {{DOMException}} with name {{NotAllowedError}}. 
+3. Set this {{DocumentOrShadowRoot}}'s adopted stylesheets to <var>adopted</var>.
+4. Now the adopted stylesheets are part of the {{DocumentOrShadowRoot}}'s style sheets and are ordered after the style sheets in {{Document/styleSheets}}, and will be considered in style calcualtion of the {{DocumentOrShadowRoot}}.


I guess we can omit this step if we add the above normative requirement.

domenic

LGTM with nits/one more substantial fix for the collection.

domenic · 2018-10-26T12:27:37Z

index.bs

@@ -90,60 +105,40 @@ dictionary CSSStyleSheetInit {
 			assign the list as <var>sheet’s</var> CSS rules;
 			otherwise,
 			set <var>sheet’s</var> CSS rules to an empty list.
-		6. If <var>sheet</var> contains one or more <a spec=css-cascade-4>@import</a> rules, throw a {{SyntaxError}}.
+		6. If <var>sheet</var> contains one or more <a spec=css-cascade-4>@import</a> rules, throw a {{NotAllowedError}} {{DOMException}}.


The convention is quotes around {{NotAllowedError}}

domenic · 2018-10-26T12:28:19Z

index.bs

 <dl>
 	<dt><dfn attribute for=DocumentOrShadowRoot lt="adoptedStyleSheets">adoptedStyleSheets</dfn></dt>
 	<dd>
-		Style sheets assigned to this attribute are part of the <a spec=cssom-1>document CSS style sheets</a>.
-		They are ordered after the stylesheets in {{Document/styleSheets}}.
+		On getting, {{adoptedStyleSheets}} returns this DocumentOrShadowRoot's [=adopted stylesheets=].


{{DocumentOrShadowRoot}}

domenic · 2018-10-26T12:28:36Z

index.bs

+
+		1. Let <var>adopted</var> be the result of converting the given value to a FrozenArray&lt;CSSStyleSheet>
+		2. If any entry of <var>adopted</var> has no associated document (e.g. it's not made by factory methods to construct stylesheets), throw a {{NotAllowedError}} {{DOMException}}. 
+		3. Set this {{DocumentOrShadowRoot}}'s adopted stylesheets to <var>adopted</var>.


[=adopted stylesheets=]

domenic · 2018-10-26T12:31:35Z

index.bs

+Every {{DocumentOrShadowRoot}} has <dfn>adopted stylesheets</dfn>.
+
+The user agent must include all style sheets in the {{DocumentOrShadowRoot}}'s
+[=adopted stylesheets=] inside its <a spec=cssom-1>document CSS style sheets</a> collection if it is a {{Document}}, or shadow CSS style sheets collection if it is a {{ShadowRoot}}. The [=adopted stylesheets=] are ordered after all the other style sheets (i.e. those derived from {{DocumentOrShadowRoot/styleSheets}}).


Hey, I figured out the solution. Just use <a>document or shadow root CSS style sheets</a> and it will link to https://drafts.csswg.org/cssom/#documentorshadowroot-document-or-shadow-root-css-style-sheets, which does what you want.

oh it was right there! Thanks.

General tidying, add motivation, clearer adoptedStyleSheets, remove d…

02ba444

…ocument restriction

rakina requested a review from tabatkins October 24, 2018 14:51

domenic reviewed Oct 26, 2018

View reviewed changes

Make it normative and resolve minor comments

8125985

domenic approved these changes Oct 26, 2018

View reviewed changes

fix nits

ea0dcc9

rakina merged commit 706508a into gh-pages Oct 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General tidying, clearer adoptedStyleSheets, remove document restriction #60

General tidying, clearer adoptedStyleSheets, remove document restriction #60

rakina commented Oct 24, 2018 •

edited by pr-preview bot

Loading

rakina commented Oct 24, 2018

domenic left a comment

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

rakina Oct 26, 2018

domenic Oct 26, 2018

domenic left a comment

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

domenic Oct 26, 2018

rakina Oct 26, 2018

		@@ -144,6 +128,17 @@ partial interface DocumentOrShadowRoot {
		</dd>
		</dl>

General tidying, clearer adoptedStyleSheets, remove document restriction #60

General tidying, clearer adoptedStyleSheets, remove document restriction #60

Conversation

rakina commented Oct 24, 2018 • edited by pr-preview bot Loading

rakina commented Oct 24, 2018

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakina commented Oct 24, 2018 •

edited by pr-preview bot

Loading