Summary: | CSS JIT: add support for the "any" pseudo class | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | CSS | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, commit-queue | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Yusuke Suzuki
2014-06-03 03:31:18 PDT
Created attachment 232422 [details]
Patch
Comment on attachment 232422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232422&action=review Added comments. > Source/WebCore/cssjit/SelectorCompiler.cpp:160 > + Vector<Vector<SelectorFragment>> anyFilters; Used ">>" since C++11 is enabled. > Source/WebCore/cssjit/SelectorCompiler.cpp:461 > + // :any() may not contain complex selectors which have combinators. simple_selectors don't contain combinators. So fragmentList.size() always becomes 1. > Source/WebCore/cssjit/SelectorCompiler.cpp:467 > + subFragment.inFunctionalPseudoClass = true; Set `inFunctionalPseudoClass` true. > Source/WebCore/cssjit/SelectorCompiler.cpp:476 > + ASSERT(!anyFragments.isEmpty()); anyFragments always has fragments. > Source/WebCore/cssjit/SelectorCompiler.cpp:2621 > + RELEASE_ASSERT(!subFragments.isEmpty()); This is guaranteed in the construction phase. > Source/WebCore/cssjit/SelectorCompiler.cpp:2632 > + // At the last fragment, optimize the failure jump to jump to the non-local failure directly. Optimized path. At the last fragment, the failure jump should jump to the non-local failure site directly. This removes unnecessary trampolines. > LayoutTests/ChangeLog:11 > + * fast/selectors/pseudo-class-any.html: Added. :hover / :active tests are needed? If so, I'll add them. Comment on attachment 232422 [details]
Patch
The patch is great. There is an issue I also forgot about :not(), fragmentOnlyMatchesLinksInQuirksMode() should return false in those cases.
Let's land this since it is complete and correct with the exception of fragmentOnlyMatchesLinksInQuirksMode(). I'll write a quick fix for fragmentOnlyMatchesLinksInQuirksMode.
Comment on attachment 232422 [details] Patch Clearing flags on attachment: 232422 Committed r169569: <http://trac.webkit.org/changeset/169569> All reviewed patches have been landed. Closing bug. (In reply to comment #3) > (From update of attachment 232422 [details]) > The patch is great. There is an issue I also forgot about :not(), fragmentOnlyMatchesLinksInQuirksMode() should return false in those cases. > Let's land this since it is complete and correct with the exception of fragmentOnlyMatchesLinksInQuirksMode(). I'll write a quick fix for fragmentOnlyMatchesLinksInQuirksMode. Oh! Thank you. I've commented your patch :) https://bugs.webkit.org/show_bug.cgi?id=133493 |