We never removed the event listeners (the code was there, lol, but the function
that was supposed to call into the tooltip listener returned
NS_ERROR_NOT_IMPLEMENTED instead).
Furthermore, we added an event listener each time we reframed an element, which
is insane. Basically, each time an element with tooltip / tooltiptext gets its
frame tree reconstructed, we add the even listener, again, and we never free it.
Xidorn pointed out that this is not such a huge deal because we deduplicate
event listeners per spec, but still...
Move the code from the RestyleManager and the frame constructor to AfterSetAttr
/ BindToTree / UnbindFromTree in nsXULElement to hopefully make this saner.
MozReview-Commit-ID: 6BQbIQJ87qt
We just need to use the existing StyleChildrenIterator which iterates over them.
We need to be a bit careful though, since ::before and ::after are owned by
their own frame, and thus could be unbound from the tree or even dead after
removing the frame.
Hopefully the only access to the node being removed is unnecessary (anon roots
don't have siblings anyway).
There's also the weird thing of the thing we're iterating changing under the
hood. It works fine for this case, but maybe it would be better to handle them
explicitly like:
if (Element* before = nsLayoutUtils::GetBeforePseudo(aChild)) {
bool didReconstruct = ContentRemoved(aChild, ...);
if (didReconstruct) {
return true;
}
MOZ_ASSERT(!nsLayoutUtils::GetBeforePseudo(aChild));
}
// Same for ::after.
StyleChildrenIterator iter(aChild);
for (..) {
// Do the rest of the kids, which can't get unbound.
}
That'd repeat a bunch of code, so not a fan neither... I pointed this out more
explicitly in a comment instead.
MozReview-Commit-ID: HBsjLH01Db3
This removes the two calls to `FlushPendingLinkUpdates` in nsCSSFrameConstructor
and GeckoRestyleManager, which appear to have no effect.
Looking through what the pending link code is attempting to do:
* When a new anchor is bound in `BindToTree` we do:
1. Link::ResetLinkState(false, Link::ElementHasHref());
* Set link's mLinkState to default (either unvisited or not link)
* Set element's link mState bits to default (either unvisited or not link)
2. doc->RegisterPendingLinkUpdate(this);
* Schedules idle dispatch to run `FlushPendingLinkUpdates` within 1 sec
* In `FlushPendingLinkUpdates`:
* For each pending link, call element->UpdateLinkState(link->LinkState());
1. Register link for async history update to get potential future visited
state
2. mLinkState is still unvisited / not link until we hear from history, so
element state is unchanged
Thus, there seems to be no need to call `FlushPendingLinkUpdates` outside of
`BindToTree`, since visited state is always applied async anyway (so it doesn't
work as an optimization to avoid restyling if visited, since that will trigger
later).
MozReview-Commit-ID: ADwsYz8vNyd
Bug 1427292 broke display: contents on non-special MathML elements.
Just for reference, I've manually audited calls to nsIFrame::GetContent() in
MathML and turns out that MathML is pretty well-behaved in that sense (it
inspects the frame tree, then gets the content), so it should work fine with
display: contents / ShadowDOM.
Only exception to that is[1], but that one seems harmless.
[1]: https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/layout/mathml/nsMathMLmactionFrame.cpp#301
So we can enable or implement when the CSSWG pleases.
MozReview-Commit-ID: 8N6kiGyjE4i
It's the last thing we check before looking into the array, modulo other two
tags, so there should be no need for something more fancy.
MozReview-Commit-ID: 4Wi1fe7jBlN
I left some IgnoredErrorResults for now where people warn on failure. We could
consider adding a WarnOnError() thing or something.
MozReview-Commit-ID: L5ttZ9CGKg0
I'm pretty sure this is not a problem now, since we don't mutate the DOM from
painting, and we don't have legacy extensions anymore.
Just to confirm, I did a try run with a RELEASE_ASSERT(!CheckDOMModified()), and
it passed.
MozReview-Commit-ID: HTekD8tsz9v
Servo tries to keep this invariant that stuff under a display: none element
subtree doesn't get styled.
This invariant isn't checked in StyleNewChildRange, but we can fix this by just
doing the work after finding an insertion point, which guarantees that.
Do the same for listboxes, since we also style early for them without knowing
whether they actually have a listbox parent frame.
MozReview-Commit-ID: Glyx24wQ6qY
The whole function doesn't have much sense.
I killed its only DOM use in bug 1427511.
Now it only has two callers in nsCSSFrameConstructor, which basically only want
to know whether the children of the same node can have different flattened tree
parents.
So let's check that directly instead (checking whether the element has a binding
or a shadow root), and simplify a bit other surrounding code while at it.
Leave the XUL popup / menubar code doing the broken thing they were doing
beforehand, because it doesn't look to me like it's trivial to fix... They're
effectively assuming that the children of the menupopup end up in a single
insertion point, which is true, but doesn't need to be. Maybe they should walk
the DOM tree? Don't want to dig into that right now, since XUL insertion points
can be filtered and all that... Not fun.
Also, this removes the broken optimization that used to check
mParentFrame->GetContent()->HasChildren(), because it's pretty broken. It used
to be relevant before bug 653881, because <children> element used to not exist,
but now the insertion point at least needs to contain the <children> element all
the time.
There even used to be a XXX comment saying that the optimization didn't work,
which was removed in:
https://hg.mozilla.org/mozilla-central/rev/2d8585ec74b3
We could still check for "no insertion points", and optimize that, but it
doesn't seem worth it.
MozReview-Commit-ID: L4lspkxKENr
The whole function doesn't have much sense.
I killed its only DOM use in bug 1427511.
Now it only has two callers in nsCSSFrameConstructor, which basically only want
to know whether the children of the same node can have different flattened tree
parents.
So let's check that directly instead (checking whether the element has a binding
or a shadow root), and simplify a bit other surrounding code while at it.
Leave the XUL popup / menubar code doing the broken thing they were doing
beforehand, because it doesn't look to me like it's trivial to fix... They're
effectively assuming that the children of the menupopup end up in a single
insertion point, which is true, but doesn't need to be.
Maybe they should walk the DOM tree? Don't want to dig into that right now,
since XUL insertion points can be reordered and all that... Not fun.
MozReview-Commit-ID: L4lspkxKENr
We remove async from the DOM all the time now since bug 1389743.
We could, before this patch, recurse into frame construction in a sync way, due
to the way we handle the weird insertion cases for <fieldset>, <details>, and
<mathml>.
This patch makes those also async, making the IssueSingleInsertNotifications
condition unnecessary.
MozReview-Commit-ID: LujPaYPwA4G
Now that accessing nsIContent slots is not a blob of virtual function calls, we
should be able to unify logic here, and speed up the not-so-rare case for
chrome, while keeping the usual case fast.
MozReview-Commit-ID: 87iY5Cbhx4T
This makes the pres shell be notified as the last observer unconditionally.
In practice this doesn't matter, and it may already be the case if an iframe
goes display: none and back. In practice, the only dependency that requires this
order is that the pres shell needs to be notified _after_ the content sink, so
we don't try to enter frame construction before beginning the shell update.
That may be worth looking into, but definitely not in the scope of this bug... :)
MozReview-Commit-ID: 9WeJ5kaUtBq
This feature is intended to help Firefox frontend developers experiment with
replacing XUL content with modern flexbox. We might also eventually use
this emulation to *actually* render most or all of our legacy XUL UI.
MozReview-Commit-ID: 3g2W9o3t23H
XUL popups (i.e. FrameConstructionItem instances with mIsPopup==true) behave
like out-of-flow content -- in particular, they generate nsPlaceholderFrame
instances. So, they need the same placeholder-wrapping behavior that we have
for other out-of-flow frames inside of an emulated legacy box, in order to
satisfy our existing invariants.
MozReview-Commit-ID: KnspN4kTPnx
