Summary: | Chaining multiple :nth-child() does not work properly | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikael Jergefelt <mike> | ||||
Component: | CSS | Assignee: | Benjamin Poulain <benjamin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Major | CC: | barraclough, benjamin, kling, phiw2 | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Mikael Jergefelt
2014-09-23 11:12:43 PDT
This is a really really dumb bug, it is sad nobody reported it earlier. The register that keeps the element's position is reused, making its value useless when evaluating a following :nth-child(). That only happen in certain cases. It looks like we were very unlucky with our tests, only hitting the case that do not destroy the counter. Created attachment 238685 [details]
Patch
Comment on attachment 238685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238685&action=review > Source/WebCore/ChangeLog:13 > + 1) When A and B are possitive, the counter would be the destination of "counter - B". nit: "positive" Comment on attachment 238685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238685&action=review > Source/WebCore/cssjit/SelectorCompiler.cpp:1768 > + if (m_registerAllocator.availableRegisterCount() > 1) { Is this (plus twice below) "> 1" because we need to allocate more registers below (e.g. "LocalRegister divisorRegister(m_registerAllocator);") If so, perhaps it might simplify to just hoist the further register allocation above this point? (In reply to comment #4) > (From update of attachment 238685 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238685&action=review > > > Source/WebCore/cssjit/SelectorCompiler.cpp:1768 > > + if (m_registerAllocator.availableRegisterCount() > 1) { > > Is this (plus twice below) "> 1" because we need to allocate more registers below (e.g. "LocalRegister divisorRegister(m_registerAllocator);") > If so, perhaps it might simplify to just hoist the further register allocation above this point? It is indeed the reason, but moving the allocation of divisorRegister above creates more problems. Since idiv only works on RAX and RDX, I need to get those two in priority. If I were to allocate divisorRegister first, it may get RAX and RDX. Committed r174035: <http://trac.webkit.org/changeset/174035> |