Bug 1374247: Remove the XBL children matching hack, and assert against it. r=xidorn,francois
MozReview-Commit-ID: 9Q9WpJFczxc
--- a/layout/reftests/dom/reftest.list
+++ b/layout/reftests/dom/reftest.list
@@ -44,12 +44,11 @@
== multipleinsertionpoints-insertmultiple.xhtml multipleinsertionpoints-ref.xhtml
# test appending some nodes whose frame construction should be done lazily
# followed by appending a node that might not be done lazily
== multipleappendwithxul.xhtml multipleappendwithxul-ref.xhtml
== multipleappendwithinput.xhtml multipleappendwithinput-ref.xhtml
== multipleappendwitheditable.xhtml multipleappendwitheditable-ref.xhtml
-fails-if(styloVsGecko||stylo) == xbl-children-1.xhtml xbl-children-1-ref.xhtml # Bug 1374247
== xbl-children-2.xhtml about:blank
== xbl-children-3.xhtml xbl-children-3-ref.html
== xbl-children-4.xhtml about:blank
deleted file mode 100644
--- a/layout/reftests/dom/xbl-children-1.xhtml
+++ /dev/null
@@ -1,39 +0,0 @@
-<html xmlns="http://www.w3.org/1999/xhtml">
-<head>
-
-<bindings xmlns="http://www.mozilla.org/xbl"
- xmlns:xhtml="http://www.w3.org/1999/xhtml">
- <binding id="a">
- <content>
- <xhtml:div class="aparent">
- <xhtml:div class="a">
- <children>
- <xhtml:div class="b">
- TEXT
- </xhtml:div>
- </children>
- </xhtml:div>
- </xhtml:div>
- </content>
- </binding>
-</bindings>
-
-<style>
-
- @namespace xbl "http://www.mozilla.org/xbl";
- .a > .b { color: green; }
- .a > xbl|children > .b { text-decoration: underline; }
- .a .b { text-transform: lowercase; }
- .aparent > * > .b { background: yellow; }
-
- /* Inverse cases. */
- .a > * > xbl|children > .b { color: red !important; }
-
-</style>
-
-</head>
-
-<body>
-<div style="-moz-binding: url(#a);" />
-</body>
-</html>
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -2309,34 +2309,54 @@ enum SelectorMatchesTreeFlags {
eLookForRelevantLink = 0x1,
// Whether SelectorMatchesTree should check for, and return true upon
// finding, an ancestor element that has an eRestyle_SomeDescendants
// restyle hint pending.
eMatchOnConditionalRestyleAncestor = 0x2,
};
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+#define ASSERT_XBL_CHILDREN_HACK() do { \
+ if (MOZ_UNLIKELY(xblChildrenMatched)) { \
+ nsAutoString selectorString; \
+ aSelector->ToString(selectorString, nullptr, false); \
+ MOZ_CRASH_UNSAFE_PRINTF("XBL compat hack matched, please file a bug " \
+ "blocking bug 1374247. Selector: %s", \
+ NS_ConvertUTF16toUTF8(selectorString).get()); \
+ } \
+} while (0)
+#else
+#define ASSERT_XBL_CHILDREN_HACK() do { /* nothing */ } while (0)
+#endif
+
static bool
SelectorMatchesTree(Element* aPrevElement,
nsCSSSelector* aSelector,
TreeMatchContext& aTreeMatchContext,
SelectorMatchesTreeFlags aFlags)
{
MOZ_ASSERT(!aSelector || !aSelector->IsPseudoElement());
nsCSSSelector* selector = aSelector;
Element* prevElement = aPrevElement;
+
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+ bool xblChildrenMatched = false;
+#endif
+
while (selector) { // check compound selectors
NS_ASSERTION(!selector->mNext ||
selector->mNext->mOperator != char16_t(0),
"compound selector without combinator");
// If after the previous selector match we are now outside the
// current style scope, we don't need to match any further.
if (aTreeMatchContext.mForScopedStyle &&
!aTreeMatchContext.IsWithinStyleScopeForSelectorMatching()) {
+ ASSERT_XBL_CHILDREN_HACK();
return false;
}
// for adjacent sibling combinators, the content to test against the
// selector is the previous sibling *element*
Element* element = nullptr;
if (char16_t('+') == selector->mOperator ||
char16_t('~') == selector->mOperator) {
@@ -2367,37 +2387,37 @@ SelectorMatchesTree(Element* aPrevElemen
if (aTreeMatchContext.mForScopedStyle) {
// We are moving up to the parent element; tell the
// TreeMatchContext, so that in case this element is the
// style scope element, selector matching stops before we
// traverse further up the tree.
aTreeMatchContext.PopStyleScopeForSelectorMatching(element);
}
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
// Compatibility hack: First try matching this selector as though the
// <xbl:children> element wasn't in the tree to allow old selectors
// were written before <xbl:children> participated in CSS selector
// matching to work.
if (selector->mOperator == '>' && element->IsActiveChildrenElement()) {
Element* styleScope = aTreeMatchContext.mCurrentStyleScope;
- if (SelectorMatchesTree(element, selector, aTreeMatchContext,
- aFlags)) {
- // It matched, don't try matching on the <xbl:children> element at
- // all.
- return true;
- }
+ xblChildrenMatched |=
+ SelectorMatchesTree(element, selector, aTreeMatchContext, aFlags);
+
// We want to reset mCurrentStyleScope on aTreeMatchContext
// back to its state before the SelectorMatchesTree call, in
// case that call happens to traverse past the style scope element
// and sets it to null.
aTreeMatchContext.mCurrentStyleScope = styleScope;
}
+#endif
}
}
if (!element) {
+ ASSERT_XBL_CHILDREN_HACK();
return false;
}
if ((aFlags & eMatchOnConditionalRestyleAncestor) &&
element->HasFlag(ELEMENT_IS_CONDITIONAL_RESTYLE_ANCESTOR)) {
// If we're looking at an element that we already generated an
// eRestyle_SomeDescendants restyle hint for, then we should pretend
// that we matched here, because we don't know what the values of
// attributes on |element| were at the time we generated the
@@ -2454,24 +2474,27 @@ SelectorMatchesTree(Element* aPrevElemen
aTreeMatchContext.mCurrentStyleScope = styleScope;
}
selector = selector->mNext;
}
else {
// for adjacent sibling and child combinators, if we didn't find
// a match, we're done
if (!NS_IS_GREEDY_OPERATOR(selector->mOperator)) {
+ ASSERT_XBL_CHILDREN_HACK();
return false; // parent was required to match
}
}
prevElement = element;
}
return true; // all the selectors matched.
}
+#undef ASSERT_XBL_CHILDREN_HACK
+
static inline
void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector,
ElementDependentRuleProcessorData* data, NodeMatchContext& nodeContext,
AncestorFilter *ancestorFilter)
{
if (nodeContext.mIsRelevantLink) {
data->mTreeMatchContext.SetHaveRelevantLink();
}