Bug 137032 – Chaining multiple :nth-child() does not work properly

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Mikael Jergefelt]

nth-child ranges does not work properly in Safari on iOS8 and Mac OSX Yosemite (DP8).

Examples can be viewed at:
http://nthmaster.com

nth-child(n+4):nth-child(-n+8) and most other ranges doesn't work as they should, and select way more elements.

[Benjamin Poulain]

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.

[Benjamin Poulain]

Created attachment 238685 [details]
Patch

[Chris Dumez]

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"

[Gavin Barraclough]

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?

[Benjamin Poulain]

(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.

[Benjamin Poulain]

Committed r174035: <http://trac.webkit.org/changeset/174035>