script element: language attribute overrides type attribute when type is empty string

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: deckycoss, Assigned: deckycoss)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
according to the HTML standard, when a script element contains both a type attribute and a language attribute, the latter must be ignored, even if the value of the former is the empty string. (see step 6: https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script)

however, this test shows that the language attribute is used when the type is set to the empty string: http://w3c-test.org/html/semantics/scripting-1/the-script-element/script-language-type.html
(Assignee)

Comment 1

2 years ago
Created attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

Review commit: https://reviewboard.mozilla.org/r/64280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64280/
Attachment #8771021 - Flags: review?(bkelly)
(Assignee)

Comment 2

2 years ago
Comment on attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64280/diff/1-2/
Would it make sense to just have GetScriptType return a boolean indicating whether there is an actual type?  That way you don't have to walk the attributes twice...
(Assignee)

Comment 4

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #3)
> Would it make sense to just have GetScriptType return a boolean indicating
> whether there is an actual type?  That way you don't have to walk the
> attributes twice...

i wondered about that too. maybe GetScriptType can call GetAttr directly instead of calling GetType?
Indeed.  And then GetType() can call GetScriptType if we want to avoid the code duplication.
Comment on attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

Redirecting to Boris since he has opinions here.
Attachment #8771021 - Flags: review?(bkelly) → review?(bzbarsky)
(Assignee)

Comment 7

2 years ago
Comment on attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64280/diff/2-3/
Attachment #8771021 - Flags: review?(bzbarsky) → review+
Comment on attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

https://reviewboard.mozilla.org/r/64280/#review61360

::: dom/base/nsIScriptElement.h:55
(Diff revision 3)
>  
>    /**
>     * Content type identifying the scripting language. Can be empty, in
>     * which case javascript will be assumed.
>     */
> -  virtual void GetScriptType(nsAString& type) = 0;
> +  virtual bool GetScriptType(nsAString& type) = 0;

Please document what the return value means.

r=me
(Assignee)

Comment 9

2 years ago
Comment on attachment 8771021 [details]
Bug 1286321: always ignore language attribute when processing script element if type attribute is defined;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64280/diff/3-4/
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 11

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/264ffcf6eb58
always ignore language attribute when processing script element if type attribute is defined. r=bz
Keywords: checkin-needed

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/264ffcf6eb58
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.