- Revision
- 167585
- Author
- [email protected]
- Date
- 2014-04-20 23:26:20 -0700 (Sun, 20 Apr 2014)
Log Message
Compile the :root pseudo class and fix a related issue with :nth-child()
https://bugs.webkit.org/show_bug.cgi?id=131926
Reviewed by Andreas Kling.
Source/WebCore:
Add the :root pseudo class. This is another trivial selector, we just need to compare
the element pointer with the documentElement.
I discovered some issues with :nth-child(n) through the layout tests for ":root".
When the pseudo class nth-child could match anything, no code was generated. That decision
was taken when generating the fragments.
The specification of :nth-child() has two tests: the parent test and the counter test.
Since some fragments would not generate any code for :nth-child(n), they would succeed on the root,
which is incorrect since the root should fail the parent test.
This was fixed by moving the filtering of non-counting :nth-child() after we generate the parent
check.
We still don't generate any counter test unless required.
Test: fast/selectors/nth-child-on-root.html
* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::addPseudoClassType):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementMatching):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsRoot):
* dom/Document.h:
(WebCore::Document::documentElementMemoryOffset):
LayoutTests:
Add more test coverage that would have caught the bug with :nth-child(n).
* fast/selectors/nth-child-on-root-expected.txt: Added.
* fast/selectors/nth-child-on-root.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (167584 => 167585)
--- trunk/LayoutTests/ChangeLog 2014-04-21 06:08:05 UTC (rev 167584)
+++ trunk/LayoutTests/ChangeLog 2014-04-21 06:26:20 UTC (rev 167585)
@@ -1,5 +1,17 @@
2014-04-20 Benjamin Poulain <[email protected]>
+ Compile the :root pseudo class and fix a related issue with :nth-child()
+ https://bugs.webkit.org/show_bug.cgi?id=131926
+
+ Reviewed by Andreas Kling.
+
+ Add more test coverage that would have caught the bug with :nth-child(n).
+
+ * fast/selectors/nth-child-on-root-expected.txt: Added.
+ * fast/selectors/nth-child-on-root.html: Added.
+
+2014-04-20 Benjamin Poulain <[email protected]>
+
Add Element.matches, the standard name for webkitMatchesSelector
https://bugs.webkit.org/show_bug.cgi?id=131922
Added: trunk/LayoutTests/fast/selectors/nth-child-on-root-expected.txt (0 => 167585)
--- trunk/LayoutTests/fast/selectors/nth-child-on-root-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/selectors/nth-child-on-root-expected.txt 2014-04-21 06:26:20 UTC (rev 167585)
@@ -0,0 +1,20 @@
+Verify the nth-child() pseudo class matcher always test for the parent element. Some :nth-child pseudo selectors can skip counting the siblings, but they should never skip the parent check.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.querySelectorAll("html:nth-child(1)").length is 0
+PASS document.querySelectorAll("html:nth-child(n)").length is 0
+PASS document.querySelectorAll("html:nth-child(n+1)").length is 0
+PASS document.querySelectorAll(":root:nth-child(1)").length is 0
+PASS document.querySelectorAll(":root:nth-child(n)").length is 0
+PASS document.querySelectorAll(":root:nth-child(n+1)").length is 0
+PASS getComputedStyle(document.documentElement).backgroundColor is "rgb(255, 255, 255)"
+PASS document.querySelectorAll("svg:root").length is 0
+PASS document.querySelectorAll("svg:nth-child(1)").length is 1
+PASS document.querySelectorAll("svg:nth-child(n)").length is 1
+PASS getComputedStyle(document.querySelector("svg")).backgroundColor is "rgb(1, 2, 3)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/selectors/nth-child-on-root.html (0 => 167585)
--- trunk/LayoutTests/fast/selectors/nth-child-on-root.html (rev 0)
+++ trunk/LayoutTests/fast/selectors/nth-child-on-root.html 2014-04-21 06:26:20 UTC (rev 167585)
@@ -0,0 +1,60 @@
+<!doctype html>
+<html>
+<head>
+<script src=""
+<style>
+html {
+ background-color:white;
+}
+html:nth-child(1) {
+ background-color:rgb(1, 2, 3);
+}
+html:nth-child(n) {
+ background-color:rgb(1, 2, 3);
+}
+html:nth-child(n+1) {
+ background-color:rgb(1, 2, 3);
+}
+
+:root:nth-child(1) {
+ background-color:rgb(1, 2, 3);
+}
+:root:nth-child(n) {
+ background-color:rgb(1, 2, 3);
+}
+:root:nth-child(n+1) {
+ background-color:rgb(1, 2, 3);
+}
+
+svg:nth-child(n) {
+ background-color:rgb(1, 2, 3);
+}
+svg:root {
+ background-color:rgb(4, 5, 6);
+}
+</style>
+</head>
+<body>
+ <div style="display:none">
+ <svg><g></g></svg>
+ </div>
+</body>
+<script>
+description('Verify the nth-child() pseudo class matcher always test for the parent element. Some :nth-child pseudo selectors can skip counting the siblings, but they should never skip the parent check.');
+
+shouldBe('document.querySelectorAll("html:nth-child(1)").length', '0');
+shouldBe('document.querySelectorAll("html:nth-child(n)").length', '0');
+shouldBe('document.querySelectorAll("html:nth-child(n+1)").length', '0');
+shouldBe('document.querySelectorAll(":root:nth-child(1)").length', '0');
+shouldBe('document.querySelectorAll(":root:nth-child(n)").length', '0');
+shouldBe('document.querySelectorAll(":root:nth-child(n+1)").length', '0');
+shouldBeEqualToString('getComputedStyle(document.documentElement).backgroundColor', 'rgb(255, 255, 255)');
+
+// This svg document is not the root, ":root" should not match anything, nth-child should work.
+shouldBe('document.querySelectorAll("svg:root").length', '0');
+shouldBe('document.querySelectorAll("svg:nth-child(1)").length', '1');
+shouldBe('document.querySelectorAll("svg:nth-child(n)").length', '1');
+shouldBeEqualToString('getComputedStyle(document.querySelector("svg")).backgroundColor', 'rgb(1, 2, 3)');
+</script>
+<script src=""
+</html>
Modified: trunk/Source/WebCore/ChangeLog (167584 => 167585)
--- trunk/Source/WebCore/ChangeLog 2014-04-21 06:08:05 UTC (rev 167584)
+++ trunk/Source/WebCore/ChangeLog 2014-04-21 06:26:20 UTC (rev 167585)
@@ -1,5 +1,37 @@
2014-04-20 Benjamin Poulain <[email protected]>
+ Compile the :root pseudo class and fix a related issue with :nth-child()
+ https://bugs.webkit.org/show_bug.cgi?id=131926
+
+ Reviewed by Andreas Kling.
+
+ Add the :root pseudo class. This is another trivial selector, we just need to compare
+ the element pointer with the documentElement.
+
+ I discovered some issues with :nth-child(n) through the layout tests for ":root".
+ When the pseudo class nth-child could match anything, no code was generated. That decision
+ was taken when generating the fragments.
+
+ The specification of :nth-child() has two tests: the parent test and the counter test.
+ Since some fragments would not generate any code for :nth-child(n), they would succeed on the root,
+ which is incorrect since the root should fail the parent test.
+
+ This was fixed by moving the filtering of non-counting :nth-child() after we generate the parent
+ check.
+ We still don't generate any counter test unless required.
+
+ Test: fast/selectors/nth-child-on-root.html
+
+ * cssjit/SelectorCompiler.cpp:
+ (WebCore::SelectorCompiler::addPseudoClassType):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementMatching):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsRoot):
+ * dom/Document.h:
+ (WebCore::Document::documentElementMemoryOffset):
+
+2014-04-20 Benjamin Poulain <[email protected]>
+
Add Element.matches, the standard name for webkitMatchesSelector
https://bugs.webkit.org/show_bug.cgi?id=131922
Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (167584 => 167585)
--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-04-21 06:08:05 UTC (rev 167584)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-04-21 06:26:20 UTC (rev 167585)
@@ -190,6 +190,7 @@
void generateElementHasClasses(Assembler::JumpList& failureCases, const LocalRegister& elementDataAddress, const Vector<const AtomicStringImpl*>& classNames);
void generateElementIsLink(Assembler::JumpList& failureCases);
void generateElementIsNthChild(Assembler::JumpList& failureCases, const SelectorFragment&);
+ void generateElementIsRoot(Assembler::JumpList& failureCases);
void generateElementIsTarget(Assembler::JumpList& failureCases);
// Helpers.
@@ -319,6 +320,7 @@
return FunctionType::SimpleSelectorChecker;
case CSSSelector::PseudoClassLink:
+ case CSSSelector::PseudoClassRoot:
case CSSSelector::PseudoClassTarget:
fragment.pseudoClasses.add(type);
return FunctionType::SimpleSelectorChecker;
@@ -343,10 +345,6 @@
if (a <= 0 && b < 1)
return FunctionType::CannotMatchAnything;
- // Anything modulo 1 is zero. Unless b restrict the range, this does not filter anything out.
- if (a == 1 && (!b || (b == 1)))
- return FunctionType::SimpleSelectorChecker;
-
fragment.nthChildfilters.append(std::pair<int, int>(a, b));
if (selectorContext == SelectorContext::QuerySelector)
return FunctionType::SimpleSelectorChecker;
@@ -1168,6 +1166,9 @@
if (fragment.pseudoClasses.contains(CSSSelector::PseudoClassLink))
generateElementIsLink(failureCases);
+ if (fragment.pseudoClasses.contains(CSSSelector::PseudoClassRoot))
+ generateElementIsRoot(failureCases);
+
if (fragment.pseudoClasses.contains(CSSSelector::PseudoClassTarget))
generateElementIsTarget(failureCases);
@@ -1942,6 +1943,20 @@
Assembler::RegisterID parentElement = m_registerAllocator.allocateRegister();
generateWalkToParentElement(failureCases, parentElement);
+ Vector<std::pair<int, int>> validSubsetFilters;
+ validSubsetFilters.reserveInitialCapacity(fragment.nthChildfilters.size());
+ for (const auto& slot : fragment.nthChildfilters) {
+ int a = slot.first;
+ int b = slot.second;
+
+ // Anything modulo 1 is zero. Unless b restricts the range, this does not filter anything out.
+ if (a == 1 && (!b || (b == 1)))
+ continue;
+ validSubsetFilters.uncheckedAppend(slot);
+ }
+ if (validSubsetFilters.isEmpty())
+ return;
+
// Setup the counter at 1.
LocalRegister elementCounter(m_registerAllocator);
m_assembler.move(Assembler::TrustedImm32(1), elementCounter);
@@ -2018,7 +2033,7 @@
}
// Test every the nth-child filter.
- for (const auto& slot : fragment.nthChildfilters) {
+ for (const auto& slot : validSubsetFilters) {
int a = slot.first;
int b = slot.second;
@@ -2043,6 +2058,13 @@
}
}
+void SelectorCodeGenerator::generateElementIsRoot(Assembler::JumpList& failureCases)
+{
+ LocalRegister document(m_registerAllocator);
+ getDocument(m_assembler, elementAddressRegister, document);
+ failureCases.append(m_assembler.branchPtr(Assembler::NotEqual, Assembler::Address(document, Document::documentElementMemoryOffset()), elementAddressRegister));
+}
+
void SelectorCodeGenerator::generateElementIsTarget(Assembler::JumpList& failureCases)
{
LocalRegister document(m_registerAllocator);
Modified: trunk/Source/WebCore/dom/Document.h (167584 => 167585)
--- trunk/Source/WebCore/dom/Document.h 2014-04-21 06:08:05 UTC (rev 167584)
+++ trunk/Source/WebCore/dom/Document.h 2014-04-21 06:26:20 UTC (rev 167585)
@@ -406,6 +406,7 @@
{
return m_documentElement.get();
}
+ static ptrdiff_t documentElementMemoryOffset() { return OBJECT_OFFSETOF(Document, m_documentElement); }
Element* activeElement();
bool hasFocus() const;
