Closed
Bug 1476769
Opened 6 years ago
Closed 6 years ago
Migrate the tabpanels binding to Custom Element using getCustomInterfaceCallback
Categories
(Toolkit :: UI Widgets, task, P3)
Toolkit
UI Widgets
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file)
Once we can implement the nsIDOMXULRelatedElement interface with getCustomInterfaceCallback (Bug 1461742) and the tab-base binding is removed (Bug 1446168), I think this will be a good candidate for Custom Element conversion.
It just has a few properties / fields / methods: https://bgrins.github.io/xbl-analysis/tree/#tabpanels.
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
There's one failure on OSX: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ea9a9e24b16db88ecfe1bf2a59de308030af8fe&selectedJob=204876586. This is related to `this.children` not being complete inside the connectedCallback (somehow only one of the <browser> elements exists: https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/widget/tests/test_bug428405.xul#20).
I knew we'd start to hit some cases where childNodes weren't complete inside connectedCallback during parse (https://github.com/w3c/webcomponents/issues/619, https://github.com/w3c/webcomponents/issues/668), so I think the right fix here (and anywhere we want to access childNodes) is to use delayConnectedCallback() to wait for DOMContentLoaded.
But this looks like an especially weird bug where accessing `this.children` seems to cause the value to "stick" and never get updated. If I comment out everything else except for a `connectedCallback() { console.log(this.children); }` then I see a list with length == 1. Even if I then inspect the element later this.children still only has length == 1 even though there are two children. To that point: `this.firstElementChild` and `this.lastElementChild` point to the first and second <browser> children! If we don't `console.log(this.children)` in connectedCallback, then if I inspect the element later on this.children has the correct length == 2. I tried to make this a smaller test case, but didn't have any luck (even with that exact xul test file - I guess it may have to do with it running in the test harness). It can be reproduced with https://phabricator.services.mozilla.com/D8458?vs=on&id=23619&whitespace=ignore-most#toc and `./mach mochitest test_bug428405.xul`.
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c9dd41737f5
Migrate tabpanels to a Custom Element r=dao
Comment 5•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #3)
> There's one failure on OSX:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=3ea9a9e24b16db88ecfe1bf2a59de308030af8fe&selectedJob=2
> 04876586. This is related to `this.children` not being complete inside the
> connectedCallback (somehow only one of the <browser> elements exists:
> https://searchfox.org/mozilla-central/rev/
> 28fe656de6a022d1fc01bbd3811023fca99cf223/widget/tests/test_bug428405.xul#20).
>
> I knew we'd start to hit some cases where childNodes weren't complete inside
> connectedCallback during parse
> (https://github.com/w3c/webcomponents/issues/619,
> https://github.com/w3c/webcomponents/issues/668), so I think the right fix
> here (and anywhere we want to access childNodes) is to use
> delayConnectedCallback() to wait for DOMContentLoaded.
Yeah, this is very easy with anything that schedules a microtask checkpoint. For example given a x-custom-element which logs this.children.length on connectedCallback():
<my-custom-element>
<script>document.body;</script>
<div></div>
</my-custom-element>
the connectedCallback will log 1 instead of 2.
> But this looks like an especially weird bug where accessing `this.children`
> seems to cause the value to "stick" and never get updated. If I comment out
> everything else except for a `connectedCallback() {
> console.log(this.children); }` then I see a list with length == 1. Even if I
> then inspect the element later this.children still only has length == 1 even
> though there are two children. To that point: `this.firstElementChild` and
> `this.lastElementChild` point to the first and second <browser> children! If
> we don't `console.log(this.children)` in connectedCallback, then if I
> inspect the element later on this.children has the correct length == 2.
Yeah, so .children is created lazily, so that explains why when you inspect the element without having accessed it earlier it has the right content. Lists are invalidated in:
https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/dom/base/nsContentList.cpp#670
And related code. You may have hit a bug either in the way the parser notifies of elements (which means that function wouldn't be called), or in the invalidation logic... I've poked at it but I haven't managed to break it, even in xhtml mode.
> I tried to make this a smaller test case, but didn't have any luck (even with
> that exact xul test file - I guess it may have to do with it running in the
> test harness). It can be reproduced with
> https://phabricator.services.mozilla.com/
> D8458?vs=on&id=23619&whitespace=ignore-most#toc and `./mach mochitest
> test_bug428405.xul`.
Finding a test-case would be amazing, does what I said ring a bell about something that might be special in this case?
Comment 6•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•