Closed Bug 1410895 Opened 2 years ago Closed 2 years ago

Make XBL know about its insertion point, not insertion point parent.

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

That way we can know which insertion point to look up for distributed siblings.
Comment on attachment 8921059 [details]
Bug 1410895: Make XBL slots hold the insertion point, not the XBL parent.

https://reviewboard.mozilla.org/r/192026/#review197396

::: dom/xbl/nsBindingManager.cpp:222
(Diff revision 2)
>      }
>  
>      aContent->SetXBLBinding(nullptr, this);
>    }
>  
>    // Clear out insertion parent and content lists.

s/parent/point/

::: dom/xbl/nsXBLBinding.cpp:804
(Diff revision 2)
> -      UpdateInsertionParent(mInsertionPoints[i], mBoundElement);
> -    }
> -
> -    // Now that our inserted children no longer think they're inserted
> -    // anywhere, make sure our internal state reflects that as well.
>      ClearInsertionPoints();

It looks like when this code was added (<https://hg.mozilla.org/mozilla-central/rev/731a5fcf13289e02bbd86f06950a3b93c7357e12>) calling ClearInsertionPoints() just cleared the child arrays on the insertion points and did not update the insertion point on the elements inside those insertion points.  Which is why this code had to do the updating.

It also looks like <https://hg.mozilla.org/mozilla-central/rev/a98e26325e28> changed the behavior to have XBLChildrenElement::ClearInsertedChildren clear out the insertion parent unconditionally on the kids.  It did NOT use the same logic as UpdateInsertionParent, though: it just always set to null instead of sometimes setting to the bound element itself.  After this, the UpdateInsertionParent calls are in fact redundant...
Attachment #8921059 - Flags: review?(bzbarsky) → review+
Blake, do you know what the story was with the changes I mention in comment 4?  Why do we not need to preserve the "correct" insertion parent?
Flags: needinfo?(mrbkap)
Comment on attachment 8921066 [details]
Bug 1410895: Multiple cleanups on top.

https://reviewboard.mozilla.org/r/192036/#review198174
Attachment #8921066 - Flags: review?(bzbarsky) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1ea2429943c2
Make XBL slots hold the insertion point, not the XBL parent. r=bz
https://hg.mozilla.org/integration/autoland/rev/89e379338c15
Multiple cleanups on top. r=bz
https://hg.mozilla.org/mozilla-central/rev/1ea2429943c2
https://hg.mozilla.org/mozilla-central/rev/89e379338c15
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
I can't remember why that code was there. I've done some spelunking in the history but nothing jumped out at me. Sorry.
Flags: needinfo?(mrbkap)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.