What happened in bug 1446368 is the following: We append two items to an empty
listbox.
We can't construct lazily because this is XUL, so that goes through
IssueSingleInsertNotifications for each of the items.
When we insert the first one we call LazilyStyleNewChildRange _only on the first
sibling_, yet the listbox code tries to construct frames for the next sibling
too from CreateRows. The next sibling is unstyled, so we panic.
Instead of handling it in ContentRangeInserted but not ContentAppended, just do
it in the listbox-specific code instead, which looks slightly cleaner (though we
can't assert we're constructing async).
This should fix the case where the listbox is display: none or what not which,
combined with the patch in bug 1303605, supersede the backed out patch in
bug 1429088, which was backed out because listboxes suck.
MozReview-Commit-ID: D7UQ41S6Ras
Move the assertion to the earliest point where it can happen instead, and do it
automatically on exit if it's generated content instead of relying on manual
calls.
MozReview-Commit-ID: 5oPwXg2o22V
This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.
This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.
Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.
It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.
Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.
As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].
The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.
[1]: https://github.com/w3c/csswg-drafts/issues/2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700
MozReview-Commit-ID: JoCKnGYEleD
This method is not a virtual call, and also looks nicer.
This patch was mostly generated by a Python script, but I manually
cleaned up the code in a few places where statements didn't need to be
split across multiple lines any more.
MozReview-Commit-ID: 8JExxqSRc59
Much in the spirit of bug 1442207.
They're not only unneeded, and cheap to get, but also we call them
inconsistently with the light DOM and flattened tree parent (like ContentRemoved
for display: contents), so they're really confusing, and kind of a footgun.
MozReview-Commit-ID: 9u3Kp8Kpp5i
The code was trying to assert that we had frames constructed for all the nodes
in the parent chain, but we don't bail out in the
!GetContentInsertionFrameFor(aContainer) in the case that it's a children
element, because they actually have no insertion frame, though their children
do.
Move the LazyFC check after the insertion point check. That makes the previous
check work on the insertion point of the child, which makes it sound.
This also fixes bug 1410020, and with it a Shadow DOM test-case that was failing
because we had two sibling assigned to two different <slot>s, and the second one
wasn't getting properly flagged, and thus the second sibling never got a frame.
The other two test failures in this test are an event dispatch failure, where
the position of the target is not what the test expects (we don't account for
margin and padding). Filed that as bug 1450027.
Also, added a test for which we have wrong layout without these patches, and
that crashes with "Called Servo_Element_IsDisplayNone" with the first patch of
this bug applied but not this one, due to the bogus check mentioned above.
MozReview-Commit-ID: 6OeaVrZhTDv
This is mostly code removal, changing GetDisplayContentsStyle(..) checks by an
FFI call to Servo.
The tricky parts are:
* MaybeCreateLazily, which I fixed to avoid setting bits under display: none
stuff. This was a pre-existing problem, which was wallpapered by the
sc->IsInDisplayNoneSubtree() check, which effectively made the whole
assertion useless (see bug 1381017 for the only crashtest that hit this
though).
* ContentRemoved, where we can no longer know for sure whether the element is
actually display: contents if we're removing it as a response to a style
change. See the comment there. That kinda sucks, but that case is relatively
weird, and it's better than adding tons of complexity to handle that.
* GetParentComputedStyle, which also has a comment there. Also, this function
has only one caller now, so we should maybe try to remove it.
The different assertions after DestroyFramesForAndRestyle are changed for a
single assertion in the function itself, and the node bit used as an
optimization to avoid hashtable lookups is taken back.
MozReview-Commit-ID: AZm822QnhF9
Tag is unused.
This changes how some mixes of MathML and html get wrapped in anonymous table
boxes (in particular, it changes whether it uses a MathML or an HTML table
frame). The main thing this affects is whether the frame responds to certain
attributes. Responding to mathml attributes on its mContent when that mContent
is not a MathML element is weird. So arguably this is also more correct.
However, that seems acceptable to me, and you can already get that mixing
manually. On a few (arguably simple) manual test-cases mixing MathML and HTML
tables I couldn't manage to get the patched build to render differently.
Plus, neither our reftests nor the WPT MathML test-suite upstreamed by Fred Wang
for WebKit rely on this.
MozReview-Commit-ID: 8IV3iF5xIs0
We don't extend svg elements, except in a lone test, that isn't really impacted
by this.
I agree this should look at the frame btw, though that looks a bit out of scope
for this bug.
MozReview-Commit-ID: MbvIE5TszB
The new StaticPrefs machinery means that StylePrefs can be removed.
Note that this approach mirrors all static prefs into Rust, but I have only
updated structs.rs for the prefs that Stylo uses.
On a CLOSED TREE, since a sheriff closed the tree while I was about to land this
via autoland.
MozReview-Commit-ID: G1SY0987WJ7
This patch basically does:
* remove StyleSetHandle and its corresponding files
* revisit #includes of related header files and change correspondingly
* change nsIPresShell::mStyleSet to be UniquePtr<ServoStyleSet>
* change the creating path of ServoStyleSet to pass UniquePtr
* change other mentions of StyleSetHandle to ServoStyleSet*
* remove AsServo() calls on ServoStyleSet
Some unfortunate bits:
* some methods of (Servo)StyleSet only accepts ServoStyleSheet while
many places call into the methods with StyleSheet, so there are many
->AsServo() added to sheets
MozReview-Commit-ID: K4zYnuhOurA
The old style system used FlattenedTreeIterator for lazy frame construction.
That could not find native anonymous nodes, so we had to construct eagerly in
native anonymous subtrees. Servo uses StyleChildrenIterator for the same
purpose, and that knows about native anonymous content, so we can now do lazy
construction for it.
Also, not check the container to do lazyFC on the children, it's no longer
necessary to check for anon content, and the reason we check for XUL is because
of XBL bindings, and those are loaded for the parent already, if we're about to
construct frames for the children.
Also, assert more tightly, we don't insert NAC lazily, that makes no sense.
Well, to be fair editor does insert anonymous nodes lazily sometimes (see al the
ManualNAC machinery), but it goes through the PostRecreateFramesFor path, not
through ContentInserted / LazyFC.
MozReview-Commit-ID: 2TmRNgpWaM
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
