1268759 - make ServoStyleSet::GetContext not crash when it encounters anonymous content

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: patch

This patch gives anonymous content incorrect styles but at least doesn't crash.  (This lets us get past styling <xul:scrollbar> elements in the initial document styling.)

Comment 1

[Bobby Holley (:bholley)]

Comment on attachment 8746921 [details] [diff] [review]
patch

Review of attachment 8746921 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with those issues sorted out.

::: layout/style/ServoStyleSet.cpp
@@ +92,5 @@
> +  MOZ_ASSERT(aContent->IsElement() ||
> +             aContent->IsNodeOfType(nsINode::eTEXT));
> +
> +  // XXXheycam: Anonymous content is not present when the document
> +  // is initially styled by Servo_RestyleDocument.  Just use the

I guess that's true, but the other problem is we have no API to traverse anonymous content from Servo. Worth expanding the comment to point out both issues.

@@ +100,5 @@
> +    do {
> +      nsIContent* parent = aContent->GetBindingParent();
> +      aContent = parent->IsElement() ? parent->AsElement() : nullptr;
> +    } while (aContent && aContent->IsInAnonymousSubtree());
> +    MOZ_ASSERT(aContent, "couldn't break out of anonymous content");

Why do we bother handling the parent-as-non-element case when it will effectively just result in a MOZ_ASSERT below?

Seems like we should either just assert that the binding parent is an element, or do what I did locally:

+  while (aElement->IsInAnonymousSubtree()) {
+    aElement = aElement->GetParentElement();
+  }

Comment 2

[Bobby Holley (:bholley)]

Attachment: Make ServoStyleSet handle anonymous content incorrectly but without crashing.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6cabbc782255

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6cabbc782255