Improve <style> and <script> processing and conformance by domenic · Pull Request #3024 · whatwg/html

domenic · 2017-09-08T00:20:46Z

De-genericizes <style> and <link rel="stylesheet"> to only deal with
CSS. Fixes Should we remove <style>'s language-agnostic spec prose? #2995.
Makes type="" on <style> "obsolete but conforming", since it is always
redundant.
Makes type="(a JS MIME type)" on <script> obsolete but conforming as
well. Previously we had a "should" requirement but had not recorded it
in the centralized obsolete-but-conforming section that collects such
requirements.
Makes <style> operate on child text content. Fixes <style> should use child text content changed steps #2996.
Replaces the conformance requirement (noted in the source as
"temporary") prohibiting unmatched comment-like syntax inside <style>
with a conformance requirement to be valid CSS.
Adds pointers to It's not clear how a style sheet's CSS rules are obtained #2997.
Makes it clearer that parameters are not allowed in the content type
value for script or style. Fixes <style type>/<script type> and MIME type parameters #3022.

@annevk, and ideally @zcorpan and @sideshowbarker, to review.

Not sure if we want to add more tests. If someone thinks we should, I'd appreciate help writing them :).

* De-genericizes <style> and <link rel="stylesheet"> to only deal with CSS. Fixes #2995. * Makes type="" on <style> "obsolete but conforming", since it is always redundant. * Makes type="(a JS MIME type)" on <script> obsolete but conforming as well. Previously we had a "should" requirement but had not recorded it in the centralized obsolete-but-conforming section that collects such requirements. * Makes <style> operate on child text content. Fixes #2996. * Replaces the conformance requirement (noted in the source as "temporary") prohibiting unmatched comment-like syntax inside <style> with a conformance requirement to be valid CSS. * Adds pointers to #2997. * Makes it clearer that parameters are not allowed in the content type value for script or style. Fixes #3022.

annevk

I guess we also need to tweak <script>'s processing model to explicitly allow the empty string.

annevk · 2017-09-08T08:10:00Z

+   <li>
+    <p>If <var>element</var>'s <code data-x="attr-style-type">type</code> attribute is present, and
+    its value is not an <span>ASCII case-insensitive</span> match for the string "<code
+    data-x="">text/css</code>", return.</p>


The empty string works too in Chrome, Firefox, and Safari. (Didn't test Edge.)

annevk · 2017-09-08T08:11:36Z

+    production. Classic scripts are affected by the <code
+    data-x="attr-script-charset">charset</code>, <code data-x="attr-script-async">async</code>, and
+    <code data-x="attr-script-defer">defer</code> attributes. Authors should omit the attribute,
+    instead of redundantly giving a <span>JavaScript MIME type</span>.</p></li>


Empty string works for <script> too in all browsers.

That seems like a weird detail to include in this intro section, but maybe we can work it in.

annevk · 2017-09-08T08:12:46Z

-  not be recognized as referencing any of the scripting languages listed above.</p>
+  <p class="note">For example, "<code data-x="">text/javascript; charset=utf-8</code>" will not be
+  recognized as a <span>JavaScript MIME type</span>, and scripts marked as such will not be
+  evaluated.</p>


This doesn't work as a general statement since this will work for module scripts if specified in Content-Type as such.

annevk · 2017-09-08T08:13:51Z

+
+   <li><p>The presence of a <code data-x="attr-style-type">type</code> attribute on a
+   <code>style</code> element if its value is an <span>ASCII case-insensitive</span> match for the
+   string "<code>text/css</code>".</p></li>


Shouldn't this just be the presence of the attribute?

No; the presence of the attribute with another value is an error, not a warning. This section is specifically about warnings. Compare to the language="" bullet above this.

annevk · 2017-09-08T08:36:40Z

I also noticed you didn't update the "valid MIME type" reference for script. Including in the Attributes index. That now needs to be "valid MIME type with no parameters" assuming that's the term we're going to use for this going forward.

I'm not sure what to do about the classification of JavaScript MIME type as a MIME type since MIME type also still includes parameters. Do we need to introduce a new "MIME type with no parameters" term that specifically means a subset of the MIME type production?

annevk · 2017-09-08T08:37:17Z

(Although that wouldn't work for the classification of JavaScript MIME type I suppose since when it's used with Content-Type parameters are "fine", so it's more complicated still.)

domenic · 2017-09-08T15:30:20Z

I also noticed you didn't update the "valid MIME type" reference for script. Including in the Attributes index. That now needs to be "valid MIME type with no parameters" assuming that's the term we're going to use for this going forward.

Can you point to all the locations? I thought script did not reference valid MIME type anymore as of your recent PR.

I'm not sure what to do about the classification of JavaScript MIME type as a MIME type since MIME type also still includes parameters. Do we need to introduce a new "MIME type with no parameters" term that specifically means a subset of the MIME type production?

I don't think so. I think it's fine for "MIME type" to be used, especially in conformance requirements, as a more vague term, with the processing model or specific definitions like "JavaScript MIME type" giving the exact behavior.

Can you point to a particular location where we give bad authoring advice by using the word "MIME type"? I couldn't see any, since we only reference "JavaScript MIME type" or "text/css".

annevk · 2017-09-08T17:28:19Z

Sure:

Authors must use a valid MIME type that is not a JavaScript MIME type to denote data blocks.

The requirement that data blocks must be denoted using a valid MIME type is in place to avoid potential future collisions.

And also in the Attributes index for the type attribute:

Valid MIME type; "module"

(I only separated it out to add "module", but I didn't want to fix "valid MIME type" since that would also require changes in the above-quoted bits.)

domenic · 2017-09-08T17:34:00Z

OK. But that seems fine. They can use one with or without parameters; both work for the purposes of avoiding collisions.

sideshowbarker · 2017-09-09T02:05:51Z

Assigned myself not to claim exclusive review ownership/approval-from-me-required on this but instead just so in shows up in the right place in my gitflow so that I don’t forget. Will try to review it this weekend

annevk · 2017-09-09T09:09:24Z

@domenic it doesn't seem fine for the type attribute, since it suggests text/javascript;charset=utf-8 is fine, which it's not. It's arguably fine for the valid MIME type that is not a JavaScript MIME type, but it does seem super confusing (and it's not entirely clear to me if that actually excludes text/javascript;charset=utf-8 or not).

domenic · 2017-09-13T10:30:01Z

Pushed most changes.

@annevk, I'm still confused what you are proposing here to replace the text you quoted. I think it's pretty unambiguous as-is. Perhaps you can help by pushing a commit with your suggestion.

annevk

Just nits. I'm happy to help fix them if you agree with them.

annevk · 2017-09-13T10:31:40Z

+   <li>
+    <p>If <var>element</var>'s <code data-x="attr-style-type">type</code> attribute is present, and
+    its value is neither the empty string nor an <span>ASCII case-insensitive</span> match for the
+    string "<code data-x="">text/css</code>", return.</p>


s/the string//

then return* (as per above)

annevk · 2017-09-13T10:34:19Z

  <ul>
-    <li><p>Omitting the attribute, or setting it to a <span>JavaScript MIME type</span>, means that
-    the script is a <span>classic script</span>, to be interpreted according to the JavaScript <i
+    <li><p>Omitting the attribute, setting it to the empty string, or setting it to an <span>ASCII


Extreme nitpicking, but with this change

Authors should omit the attribute, instead of redundantly giving a JavaScript MIME type.

should arguably also be changed, lest someone argues that <script type> is fine.

Agreed. "instead of redundantly giving a JavaScript MIME type (or adding an empty type attribute)" perhaps?

annevk · 2017-09-13T10:35:54Z

-  other than JavaScript is outside the scope of this specification.</p>
+  but must not support other <span data-x="MIME type">MIME types</span> for JavaScript. User agents
+  are not required to support JavaScript. The processing model for languages other than JavaScript
+  is outside the scope of this specification.</p>


Do you agree that we should have a follow-up on this to change this somehow? It seems bad if WebAssembly wouldn't result in changes to the HTML Standard due to someone thinking we don't want to consider it.

Note that this text is pre-existing and just got re-wrapped.

I suppose we could expand on this, yeah. Perhaps a call-out to WebAssembly in particular.

Yeah, I didn't mean for this to block this PR on, I just noticed it in passing and wanted to do due diligence.

annevk · 2017-09-13T10:36:29Z

@@ -58502,8 +58477,9 @@ o............A....e
  order of parameters matters is left undefined until such time as it matters; so far the only
  relevant parameter was 'e4x', and it's gone for now -->


Maybe remove this while here?

annevk · 2017-09-13T10:37:06Z


+  <p>Authors should not specify a value for the <code data-x="attr-script-type">type</code>
+  attribute on <code>script</code> elements that is an <span>ASCII case-insensitive</span> match for
+  a <span>JavaScript MIME type</span>. Instead, they should omit the attribute, which has the same


or the empty string?

Probably "the empty string or an ASCII case..." to avoid ambiguity about it being an ACIM for the empty string.

annevk · 2017-09-13T10:37:58Z

+
+   <li><p>The presence of a <code data-x="attr-style-type">type</code> attribute on a
+   <code>style</code> element if its value is an <span>ASCII case-insensitive</span> match for the
+   string "<code>text/css</code>".</p></li>


s/the string//

annevk · 2017-09-13T10:42:10Z

For the script element type attribute summary, maybe something like:

The empty string or a JavaScript MIME type; "module"; a valid MIME type that is not a JavaScript MIME type

And then at some point we define what "is not" means for MIME types.

domenic · 2017-09-13T10:46:35Z

Hmm do we even list discouraged values in the attribute summary? We don't list language="" at all... In which case maybe just the latter two? Or maybe that's too confusing.

annevk · 2017-09-13T10:50:18Z

I'd be okay with that, it's a summary after all and we do indeed seem to restrict it to things that are fully conforming.

annevk · 2017-09-13T11:57:55Z

Pushed a fixup.

domenic

Changes LGTM, thanks for picking up the slack here. Let me try to work on some tests for some of the edge cases we clarified here.

I will also file a bug on the checker.

domenic · 2017-09-14T03:39:40Z

+   <li>
+    <p>If <var>element</var>'s <code data-x="attr-style-type">type</code> attribute is present and
+    its value is neither the empty string nor an <span>ASCII case-insensitive</span> match for
+    "<code>text/css</code>", then return.</p>


Firefox also allows whitespace to surround it, interestingly. I think it's correct to disallow.

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

domenic · 2017-09-14T03:44:53Z

Tests ready: web-platform-tests/wpt#7344 @annevk feel free to approve and merge if you're happy with things.

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

domenic added document conformance topic: script topic: style labels Sep 8, 2017

annevk reviewed Sep 8, 2017

View reviewed changes

sideshowbarker self-assigned this Sep 9, 2017

Address most code review comments

c357f51

annevk reviewed Sep 13, 2017

View reviewed changes

annevk mentioned this pull request Sep 13, 2017

WebAssembly is in scope #3037

Closed

address my own nits

b58f26c

domenic commented Sep 14, 2017

View reviewed changes

domenic mentioned this pull request Sep 14, 2017

<style> and <script> conformance updates validator/validator#560

Closed

domenic commented Sep 14, 2017

View reviewed changes

domenic added a commit to web-platform-tests/wpt that referenced this pull request Sep 14, 2017

Expand type="" tests for <script> and <style>

785f9ca

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

domenic mentioned this pull request Sep 14, 2017

Expand type="" tests for <script> and <style> web-platform-tests/wpt#7344

Merged

annevk approved these changes Sep 14, 2017

View reviewed changes

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 14, 2017

Expand type="" tests for <script> and <style>

65880c6

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

annevk merged commit 9c612ac into master Sep 14, 2017

annevk deleted the script-style-tweaks branch September 14, 2017 09:42

rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this pull request Nov 8, 2017

Expand type="" tests for <script> and <style>

66bace1

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this pull request Nov 16, 2017

Expand type="" tests for <script> and <style>

554714b

Follows the clarifications in whatwg/html#3024. Also renames, re-titles, and re-jiggers some existing script tests.

		@@ -58502,8 +58477,9 @@ o............A....e
		order of parameters matters is left undefined until such time as it matters; so far the only
		relevant parameter was 'e4x', and it's gone for now -->

Conversation

domenic commented Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk commented Sep 8, 2017

Uh oh!

annevk commented Sep 8, 2017

Uh oh!

domenic commented Sep 8, 2017

Uh oh!

annevk commented Sep 8, 2017

Uh oh!

domenic commented Sep 8, 2017

Uh oh!

sideshowbarker commented Sep 9, 2017

Uh oh!

annevk commented Sep 9, 2017

Uh oh!

domenic commented Sep 13, 2017

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk commented Sep 13, 2017

Uh oh!

domenic commented Sep 13, 2017

Uh oh!

annevk commented Sep 13, 2017

Uh oh!

annevk commented Sep 13, 2017

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domenic commented Sep 14, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

domenic commented Sep 8, 2017 •

edited

Loading