We don't need the parent style context, nor the pseudo-element dance or anything
like that. All end up in the same place, Servo_ResolveStyle.
We cannot assert for text style resolution because of non-lazy frame
construction, and we cannot remove non-lazy frame construction because of XBL.
This is effectively the same code, since the old code passed the parent style
around from the frame tree / display contents map, which didn't have a similar
assertion either... Slow steps.
I'll improve and cleanup LazyFC in bug 1447506.
MozReview-Commit-ID: Ck4RCoFLGOi
The nsCSSFrameConstructor bits are now handled in PresShell::Destroy along with
the other refresh driver observers.
I cleaned up the nsRefreshDriver methods because they were using infallible
append anyway, and that simplified the logic.
MozReview-Commit-ID: 1eDUUXjUUS9
Summary: It uses two node bits that can be better suited for something else.
Reviewers: xidorn, smaug
Bug #: 1444905
Differential Revision: https://phabricator.services.mozilla.com/D709
MozReview-Commit-ID: HIPDtHm6xpM
Unfortunately this means that we need to export a couple more headers, but that
should be ok.
In particular, we have to export some headers that are #included by
nsCSSFrameConstructor.h, because nsCSSFrameConstructor.h itself will now be
included in more places outside of layout/, by .cpp files that don't necessarily
have the ability to indirectly #include its other headers, unless we export
them.
MozReview-Commit-ID: 2n9KHW6Yjrd
It doesn't make sense, since they have no frame themselves, and it breaks
invariants other code relies on. Use the parent frame instead.
The stack overflow happens because we give the first-letter frame to the
display: contents element, then we reframe it.
Removing a display: contents node calls ContentRemoved on all the children. One
of these children is this text-node inside the first-letter frame. Since it was
split by bidi resolution we go ahead and reframe the parent in:
https://searchfox.org/mozilla-central/rev/d2b4b40901c15614fad2fa34718eea428774306e/layout/base/nsCSSFrameConstructor.cpp#9688
But the parent is the display: contents node, which results in infinite
recursion.
The usage of GetParent() is wrong anyway too, since it doesn't handle XBL or
Shadow DOM in any way.
MozReview-Commit-ID: JFD16at316V
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
