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)

task

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.
Priority: -- → P3
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
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
(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?
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: