Bug 1324618 part 7. Set our new style context on all our continuations in ServoRestyleManager. r?emilio
MozReview-Commit-ID: 4gVXPDCPZnq
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -336,19 +336,18 @@ ServoRestyleManager::ProcessPostTraversa
ClearRestyleStateFromSubtree(aElement);
return true;
}
// TODO(emilio): We could avoid some refcount traffic here, specially in the
// ServoComputedValues case, which uses atomic refcounting.
//
// Hold the old style context alive, because it could become a dangling
- // pointer during the replacement. In practice it's not a huge deal (on
- // GetNextContinuationWithSameStyle the pointer is not dereferenced, only
- // compared), but better not playing with dangling pointers if not needed.
+ // pointer during the replacement. In practice it's not a huge deal, but
+ // better not playing with dangling pointers if not needed.
RefPtr<nsStyleContext> oldStyleContext =
styleFrame ? styleFrame->StyleContext() : nullptr;
UndisplayedNode* displayContentsNode = nullptr;
// FIXME(emilio, bug 1303605): This can be simpler for Servo.
// Note that we intentionally don't check for display: none content.
if (!oldStyleContext) {
displayContentsNode =
@@ -384,24 +383,27 @@ ServoRestyleManager::ProcessPostTraversa
nsIAtom* pseudoTag = pseudo == CSSPseudoElementType::NotPseudo
? nullptr : nsCSSPseudoElements::GetPseudoAtom(pseudo);
newContext = aRestyleState.StyleSet().GetContext(
computedValues.forget(), aParentContext, pseudoTag, pseudo, aElement);
newContext->EnsureSameStructsCached(oldStyleContext);
- // XXX This could not always work as expected: there are kinds of content
- // with the first split and the last sharing style, but others not. We
- // should handle those properly.
- // XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
- // right, but not other cases where we happen to have different styles on
- // different continuations... (e.g. first-line).
- for (nsIFrame* f = styleFrame; f;
- f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+ // We want to walk all the continuations here, even the ones with different
+ // styles. In practice, the only reason we get continuations with different
+ // styles here is ::first-line (::first-letter never affects element
+ // styles). But in that case, newContext is the right context for the
+ // _later_ continuations anyway (the ones not affected by ::first-line), not
+ // the earlier ones, so there is no point stopping right at the point when
+ // we'd actually be setting the right style context.
+ //
+ // This does mean that we may be setting the wrong style context on our
+ // initial continuations; ::first-line fixes that up after the fact.
+ for (nsIFrame* f = styleFrame; f; f = f->GetNextContinuation()) {
f->SetStyleContext(newContext);
}
if (MOZ_UNLIKELY(displayContentsNode)) {
MOZ_ASSERT(!styleFrame);
displayContentsNode->mStyle = newContext;
}
@@ -488,18 +490,28 @@ ServoRestyleManager::ProcessPostTraversa
if (!primaryFrame) {
return false;
}
RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
nsStyleContext& newContext = aPostTraversalState.ComputeStyle(aTextNode);
aPostTraversalState.ComputeHintIfNeeded(aTextNode, primaryFrame, newContext);
- for (nsIFrame* f = primaryFrame; f;
- f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+ // We want to walk all the continuations here, even the ones with different
+ // styles. In practice, the only reasons we get continuations with different
+ // styles are ::first-line and ::first-letter. But in those cases,
+ // newContext is the right context for the _later_ continuations anyway (the
+ // ones not affected by ::first-line/::first-letter), not the earlier ones,
+ // so there is no point stopping right at the point when we'd actually be
+ // setting the right style context.
+ //
+ // This does mean that we may be setting the wrong style context on our
+ // initial continuations; ::first-line/::first-letter fix that up after the
+ // fact.
+ for (nsIFrame* f = primaryFrame; f; f = f->GetNextContinuation()) {
f->SetStyleContext(&newContext);
}
return true;
}
void
ServoRestyleManager::ClearSnapshots()
--- a/layout/generic/nsInlineFrame.cpp
+++ b/layout/generic/nsInlineFrame.cpp
@@ -1027,46 +1027,52 @@ nsInlineFrame::UpdateStyleOfOwnedAnonBox
"Only the primary frame of the inline in a block-inside-inline "
"split should have NS_FRAME_OWNS_ANON_BOXES");
MOZ_ASSERT(mContent->GetPrimaryFrame() == this,
"We should be the primary frame for our element");
nsIFrame* blockFrame = GetProperty(nsIFrame::IBSplitSibling());
MOZ_ASSERT(blockFrame, "Why did we have an IB split?");
+ // The later inlines need to get our style.
+ nsStyleContext* ourStyle = StyleContext();
+
// The anonymous block's style inherits from ours, and we already have our new
// style context.
RefPtr<nsStyleContext> newContext =
aRestyleState.StyleSet().ResolveInheritingAnonymousBoxStyle(
- nsCSSAnonBoxes::mozBlockInsideInlineWrapper, StyleContext());
+ nsCSSAnonBoxes::mozBlockInsideInlineWrapper, ourStyle);
// We're guaranteed that newContext only differs from the old style context on
// the block in things they might inherit from us. And changehint processing
// guarantees walking the continuation and ib-sibling chains, so our existing
// changehint being in aChangeList is good enough. So we don't need to touch
// aChangeList at all here.
while (blockFrame) {
MOZ_ASSERT(!blockFrame->GetPrevContinuation(),
"Must be first continuation");
MOZ_ASSERT(blockFrame->StyleContext()->GetPseudo() ==
nsCSSAnonBoxes::mozBlockInsideInlineWrapper,
"Unexpected kind of style context");
- // We _could_ just walk along using GetNextContinuationWithSameStyle here,
- // but it would involve going back to the first continuation every so often,
- // which is a bit silly when we can just keep track of our first
- // continuations.
+ // We don't want to just walk through using GetNextContinuationWithSameStyle
+ // here, because we want to set updated style contexts on both our
+ // ib-sibling blocks and inlines.
for (nsIFrame* cont = blockFrame; cont; cont = cont->GetNextContinuation()) {
cont->SetStyleContext(newContext);
}
nsIFrame* nextInline = blockFrame->GetProperty(nsIFrame::IBSplitSibling());
MOZ_ASSERT(nextInline, "There is always a trailing inline in an IB split");
+
+ for (nsIFrame* cont = nextInline; cont; cont = cont->GetNextContinuation()) {
+ cont->SetStyleContext(ourStyle);
+ }
blockFrame = nextInline->GetProperty(nsIFrame::IBSplitSibling());
}
}
//////////////////////////////////////////////////////////////////////
// nsLineFrame implementation
new file mode 100644
--- /dev/null
+++ b/layout/reftests/ib-split/insert-into-split-inline-17-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<style>
+ span { color: green; }
+</style>
+<span>
+ <div></div>
+ First Second Third
+ <div></div>
+</span>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/ib-split/insert-into-split-inline-17.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<style>
+ #target { color: red; }
+ #target.green { color: green; }
+</style>
+<span id="target">
+ <div></div>
+ First <span id="next"> Third</span>
+ <div></div>
+</span>
+<script>
+ onload = function() {
+ var s = document.querySelector("#target");
+ window.color = getComputedStyle(s).color;
+ s.className = "green";
+ window.newColor = getComputedStyle(s).color;
+ s.insertBefore(document.createTextNode("Second"),
+ s.querySelector("#next"));
+ }
+</script>
--- a/layout/reftests/ib-split/reftest.list
+++ b/layout/reftests/ib-split/reftest.list
@@ -59,16 +59,17 @@
== insert-into-split-inline-13-ref.html insert-into-split-inline-13-noib-ref.html
== insert-into-split-inline-14.html insert-into-split-inline-14-ref.html
== insert-into-split-inline-14-ref.html insert-into-split-inline-14-noib-ref.html
== insert-into-split-inline-15.html insert-into-split-inline-15-ref.html
== insert-into-split-inline-15-ref.html insert-into-split-inline-15-noib-ref.html
== insert-into-split-inline-16a.html insert-into-split-inline-16-ref.html
== insert-into-split-inline-16b.html insert-into-split-inline-16-ref.html
== insert-into-split-inline-16-ref.html insert-into-split-inline-16-noib-ref.html
+== insert-into-split-inline-17.html insert-into-split-inline-17-ref.html
== float-inside-inline-between-blocks-1.html float-inside-inline-between-blocks-1-ref.html
== table-pseudo-in-part3-1.html table-pseudo-in-part3-1-ref.html
== emptyspan-1.html emptyspan-1-ref.html
== emptyspan-2.html emptyspan-2-ref.html
== emptyspan-3.html emptyspan-3-ref.html
== emptyspan-4.html emptyspan-4-ref.html
== split-inner-inline-1.html split-inner-inline-1-ref.html
== split-inner-inline-2.html split-inner-inline-2-ref.html