1318630 - Crash in nsCSSFrameConstructor::FindInputData with two-argument form of document.createElement() (dom.webcomponents.enabled=true)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Sebastian Rittau]

Using Firefox 50.0 with dom.webcomponents.enabled set to true, the following JavaScript code will hard-crash the browser:

    class MyElement extends HTMLInputElement {}

    document.registerElement("my-element", {
        prototype: MyElement.prototype,
        extends: "input"
    });


    const element = document.createElement("input", {is: "my-element"});
    document.body.appendChild(element);

If you replace the last line by:

    element.type = "number";

you get the error message "TypeError: 'set type' called on an object that does not implement interface HTMLInputElement."

Using innerHTML will work fine:

    const container = document.createElement("div");
    container.innerHTML = '<input is="my-element"/>';
    const element = container.firstChild;
    element.type = "number";
    document.body.appendChild(element);

This problem is reproducible in Firefox 50.0, on Debian Linux (package 50.0-1) and on Windows 7 using the official Firefox download. Yesterday, it was also reproducible using a Firefox 51 Beta version on Windows.

Comment 1

[Loic]

Indeed, this JS code crashes Firefox (50/53) in non-e10s mode, but not in e10s which throws exception.

https://crash-stats.mozilla.com/report/index/f8f801f2-1e0f-4e2b-8df6-a04812161118

Comment 2

[Loic]

Reg range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=db0243f476257
6ce4874d56bd3c92c56fcf78ab7&tochange=fc1d44b2cb7b7d7c47a365bddfc23ff991505003

Jocelyn, can you check this regression crash after your patches in bug 1276579.

You need to disable e10s to reproduce the crash in Nightly.

Comment 3

[Sebastian Rittau]

Please also see bug 1318729, which also involved a crash using Web Components and FF 50. They might be related.

Comment 4

[Marcia Knous [:marcia]]

Tracking 53+ for this reproducible crash.

Comment 5

[Julien Cristau [:jcristau]]

low volume crash with non default settings, not tracking for 52

Comment 6

[Astley Chen (inactive)]

Edgar, is this bug still in your radar?

Comment 7

[Gerry Chang [:gchang]]

Track 51- and mark 51 fix-optional as the volume of crash is low.

Comment 8

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

The crash could not be reproduced with dom.webcomponents.customelements.enabled set to true. I suspect there is something wrong on handling feature control property.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

very low volume crash, wontfix for 50.1.0 release

Comment 10

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Edgar Chen [:edgar][:echen] from comment #8)
> The crash could not be reproduced with
> dom.webcomponents.customelements.enabled set to true. I suspect there is
> something wrong on handling feature control property.

Beside the feature control property, we always create HTMLElement for custom element [1], so we got "TypeError: 'set type' called on an object that does not implement interface HTMLInputElement." when access the attribute of input element.

[1] http://searchfox.org/mozilla-central/rev/5ee2bd8800b007d6c120d9521d5bf01131885afb/dom/html/nsHTMLContentSink.cpp#256-268

Comment 11

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Part 1: Fix missing control pref checks for custom element feature, v1

Comment 12

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Part 2: Create correct html element object when "is" is specified for custom element, v1

Comment 13

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bfb123aaadd8372b54bb552994796d2374bd32e&filter-tier=1&group_state=expanded

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ce261714f79
Part 1: Fix missing control pref checks for custom element feature; r=wchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/38bc75a0a89e
Part 2: Create correct html element object when "is" is specified for custom element; r=wchen

Comment 15

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

This also happens on e10s, so I modified the bug description to reflect the fact more.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7ce261714f79
https://hg.mozilla.org/mozilla-central/rev/38bc75a0a89e

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance (assuming you feel the risk is sufficiently low for doing so).

Comment 18

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8818179 [details] [diff] [review]
Part 1: Fix missing control pref checks for custom element feature, v1

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1276579.
[User impact if declined]: Browser/Tab crash when enabling and using custom element feature.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: custom element feature is preffed off by default on aurora.
[String changes made/needed]: None.

Comment 19

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8818181 [details] [diff] [review]
Part 2: Create correct html element object when "is" is specified for custom element, v1

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1276579.
[User impact if declined]: Browser/Tab crash when enabling and using custom element feature.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: custom element feature is preffed off by default on aurora.
[String changes made/needed]: None.

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 8818179 [details] [diff] [review]
Part 1: Fix missing control pref checks for custom element feature, v1

fix crash with custom element feature on aurora52

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 8818181 [details] [diff] [review]
Part 2: Create correct html element object when "is" is specified for custom element, v1

fix crash with custom element feature on aurora52

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6a7b2a984a1a
https://hg.mozilla.org/releases/mozilla-aurora/rev/f724a0c2c3d7

Comment 23

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: [beta] Part 1: Fix missing control pref checks for custom element feature, v1, r=wchen

Part 1 patch for beta.

Comment 24

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: [beta] Part 2: Create correct html element object when "is" is specified for custom element, v1, r=wchen

Part 2 patch for beta

Comment 25

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8823491 [details] [diff] [review]
[beta] Part 1: Fix missing control pref checks for custom element feature, v1, r=wchen

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1276579.
[User impact if declined]: Browser/Tab crash when enabling and using custom element feature.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: custom element feature is preffed off by default on beta.
[String changes made/needed]: None.

Comment 26

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8823493 [details] [diff] [review]
[beta] Part 2: Create correct html element object when "is" is specified for custom element, v1, r=wchen

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1276579.
[User impact if declined]: Browser/Tab crash when enabling and using custom element feature.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: custom element feature is preffed off by default on beta.
[String changes made/needed]: None.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8823491 [details] [diff] [review]
[beta] Part 1: Fix missing control pref checks for custom element feature, v1, r=wchen

This isn't a high volume crash on release (5 crashes in a month)
But given that it only would happen with non default prefs, why not try it if the behavior is more correct.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8823493 [details] [diff] [review]
[beta] Part 2: Create correct html element object when "is" is specified for custom element, v1, r=wchen

This should land for 51 beta 13 next week.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

This is hitting conflicts due to bug 1309140. Not sure if you want to just rebase around it for the Beta uplift or get that renaming uplifted too.

Comment 30

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #29)
> This is hitting conflicts due to bug 1309140. Not sure if you want to just
> rebase around it for the Beta uplift or get that renaming uplifted too.

Attachment 8823491 [details] [diff] and Attachment 8823493 [details] [diff] are the patches rebased for the Beta uplift. Please let me know if they still hit the conflicts, thank you.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Hah, total fail on my part there!

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a149e1c2822
https://hg.mozilla.org/releases/mozilla-beta/rev/2970d3bf6664