Closed
Bug 1454157
Opened 6 years ago
Closed 6 years ago
Listboxes suck, and layout/base/crashtests/414175-1.xul breaks invariants.
Categories
(Core :: XBL, enhancement)
Core
XBL
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(2 files)
With https://hg.mozilla.org/integration/autoland/rev/b12b7c5b8178, it asserts on teardown, meaning that something is broken. I tried to land this patch in bug 1453206, but failed because of this single crashtest.
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(bugs)
Assignee | ||
Comment 1•6 years ago
|
||
Blerg, meant to a self-ni?, sorry.
Flags: needinfo?(bugs) → needinfo?(emilio)
Assignee | ||
Comment 2•6 years ago
|
||
Ugh, this is a weird XBL bug, we're losing track of the inserted kid somewhere: (rr) p node $28 = (const nsXULElement *) 0x7f9e87c4b1f0 (rr) p $28->mSlots->mExtendedSlots->mXBLInsertionPoint.mRawPtr $29 = (mozilla::dom::XBLChildrenElement *) 0x7f9e87c43520 (rr) p $29->mInsertedChildren $30 = nsTArray<nsIContent*> = {0x7f9e87cbd940}
Assignee | ||
Updated•6 years ago
|
Component: CSS Parsing and Computation → XBL
Assignee | ||
Comment 3•6 years ago
|
||
I cannot make any sense of the MaybeRemoveDefaultContent() bits in: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/dom/xbl/XBLChildrenElement.h#58 Like... We first insert, _then_ check and only if there's no inserted children we remove the insertion point. Which is clearly bogus.
Assignee | ||
Comment 4•6 years ago
|
||
That of course fixes the inconsistency issue. Yet that still doesn't fix the fact that we don't tell layout about it, so with that fixed the assertion will still happen.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=22e521879343fce3bf76e9adeadc22d896e2c481
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8967954 [details] Bug 1454157: Fix default content removal to actually work. https://reviewboard.mozilla.org/r/236648/#review242422
Attachment #8967954 -
Flags: review+
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8967955 [details] Bug 1454157: Tell layout about default content going away. https://reviewboard.mozilla.org/r/236650/#review242424 ::: dom/xbl/nsXBLBinding.cpp:350 (Diff revision 1) > > // Insert explicit children into insertion points > if (mDefaultInsertionPoint && mInsertionPoints.IsEmpty()) { > ExplicitChildIterator iter(mBoundElement); > for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) { > - mDefaultInsertionPoint->AppendInsertedChild(child); > + mDefaultInsertionPoint->AppendInsertedChild(child, false); Please add a comment why false is passed here (since we're just creating the whole anon content) ::: dom/xbl/nsXBLBinding.cpp:360 (Diff revision 1) > // node has any non <xul:template> or <xul:observes> children. > ExplicitChildIterator iter(mBoundElement); > for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) { > XBLChildrenElement* point = FindInsertionPointForInternal(child); > if (point) { > - point->AppendInsertedChild(child); > + point->AppendInsertedChild(child, false); and here too
Attachment #8967955 -
Flags: review?(bugs) → review+
Comment 10•6 years ago
|
||
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d4ea74f174c0 Fix default content removal to actually work. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/8fdb38ff02b6 Tell layout about default content going away. r=smaug
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d4ea74f174c0 https://hg.mozilla.org/mozilla-central/rev/8fdb38ff02b6
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8967954 [details] Bug 1454157: Fix default content removal to actually work. https://reviewboard.mozilla.org/r/236648/#review242716
Attachment #8967954 -
Flags: review?(mrbkap) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•