Bug 136914

Summary: ARMv7(s) Assembler: LDRH with immediate offset is loading from the wrong offset
Product: WebKit Reporter: alpha
Component: CSSAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, barraclough, benjamin, cdumez, rob, sam
Priority: P2    
Version: 525.x (Safari 3.2)   
Hardware: iPhone / iPad   
OS: Other   
URL: http://www.bronco.co.uk/
Bug Depends on: 140937    
Bug Blocks:    
Attachments:
Description Flags
Shows lack of margin between individual column
none
Patch none

Description alpha 2014-09-18 02:01:57 PDT
On an iPad 3 with iOS 8 the margin set to an nth-child is applied to all children. The margins appear normal on refresh and then update to remove margins.

The associated CSS is

@media screen and (min-width:820px){
    .services--main li{margin-left:5.660377358490566%;width:20.75471698113208%}
    .services--main li:nth-child(4n+1){clear:both;margin-left:0}
}

and relates to the 8 icon blocks on the homepage underneath the heading 'We are a full service digital agency specialising in...'

I can't find any other contributing factors or replicate the same issue on other websites or in other browsers including the latest version of Chrome on iOS.
Comment 1 Benjamin Poulain 2014-09-18 02:16:35 PDT
(In reply to comment #0)
> I can't find any other contributing factors or replicate the same issue on other websites or in other browsers including the latest version of Chrome on iOS.

This point toward a compiler bug. Chrome uses UIWebView, which does not support the CSS JIT.

I suspect it is something wrong in the JIT compilation of modulo for that series of CPU. I'll check that.

Do you need a workaround?
Comment 2 alpha 2014-09-18 02:30:56 PDT
Ref:workaround. I don't think so, the issue isn't adversely affecting user experience enough to require this. Not if the issue is related to a specific device and OS combination rather than multiple/all iOS8 devices.
Comment 3 Benjamin Poulain 2014-09-18 18:17:42 PDT
I just tested with an iPad 4th generation and I was unable to reproduce the problem.

Next I'll set up a 3rd gen for testing.
Comment 4 Benjamin Poulain 2014-09-18 21:12:54 PDT
Hum, I can't reproduce on iPad 3rd generation either, ":nth-child(4n+1)" works fine.

Can you please give the exact revision number you are testing on? Settings->General->About->Version.

Do you have any other information that would be useful for testing?

Can you attach a screenshot of the website on your device?
Comment 5 alpha 2014-09-19 01:13:46 PDT
Created attachment 238354 [details]
Shows lack of margin between individual column

Version is 8.0(12A365)

In some instances the margins do show but there's no pattern to it to indicate an clear reason as to when margins do or do not show.
Comment 6 Benjamin Poulain 2014-09-19 13:02:43 PDT
Ok, got it on 12A365. It is not a selector bug as I thought, it looks a style sharing bug. This is gonna affect all devices. :(

There is a high probability this is fixed in WebKit trunk. I have completely changed the style invalidation of :nth-child() a couple of weeks ago.

I am currently changing :nth-child() for CSS 4, I should retest the bug after that work is done.
Comment 7 Benjamin Poulain 2014-09-26 10:05:14 PDT
Kevin Muncie sent me this: http://stackoverflow.com/questions/26032513/ios8-safari-after-a-pushstate-the-nth-child-selectors-not-works

It looks like a style marking bug specific to ARMv7.
Comment 8 Benjamin Poulain 2014-11-12 21:07:41 PST
Created attachment 241466 [details]
Patch
Comment 9 Michael Saboff 2014-11-12 22:32:55 PST
Comment on attachment 241466 [details]
Patch

r=me
Comment 10 Geoffrey Garen 2014-11-13 11:03:57 PST
Comment on attachment 241466 [details]
Patch

Can you write a regression test for this?
Comment 11 Benjamin Poulain 2014-11-13 12:43:27 PST
Comment on attachment 241466 [details]
Patch

Clearing flags on attachment: 241466

Committed r176083: <http://trac.webkit.org/changeset/176083>
Comment 12 Benjamin Poulain 2014-11-13 12:43:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Benjamin Poulain 2014-11-13 12:45:34 PST
(In reply to comment #10)
> Comment on attachment 241466 [details]
> Patch
> 
> Can you write a regression test for this?

I'll explain this to you when you are back to your office :)
Comment 14 rob 2015-02-23 10:11:51 PST
Is there possibly a regression with this between 8.1 and 8.1.3? I found this issue while searching for solutions to an issue I'm seeing in our CMS templates' rendering code where, intermittently, properties set to an nth-child seem to be applied to all children (in this case, clear:both on every child of a floated grid system, where nth-child styles intend to apply clear:both to only the first child in a new row). I see this in physical iPads running 8.1.3 but cannot reproduce in the iOS 8.1 simulator (XCode seems to not allow updating the simulator all the way to 8.1.3 for now).

Orientation change sometimes but not always resolves the issue. Inspecting an affected page and forcing a redraw on a parent element of this grid usually makes the issue go away; in fact, simply clicking into the CSS editing fields for affected grid items in the Safari Web Inspector often makes the issue go away instantly.

One affected page is http://orbit.spacecrafted.com.
Comment 15 Benjamin Poulain 2015-02-23 10:17:17 PST
(In reply to comment #14)
> Is there possibly a regression with this between 8.1 and 8.1.3? I found this
> issue while searching for solutions to an issue I'm seeing in our CMS
> templates' rendering code where, intermittently, properties set to an
> nth-child seem to be applied to all children (in this case, clear:both on
> every child of a floated grid system, where nth-child styles intend to apply
> clear:both to only the first child in a new row). I see this in physical
> iPads running 8.1.3 but cannot reproduce in the iOS 8.1 simulator (XCode
> seems to not allow updating the simulator all the way to 8.1.3 for now).

I don't remember any related change between 8.1 and 8.1.3.

This bug only affects 32 bits ARM devices, you would not see the problem in the simulator.
Comment 16 rob 2015-02-23 10:40:04 PST
Thanks Benjamin — so, any recommendations as to how I can determine if the issue I see on the site in question is actually related to this bug or not? Based on the date this patch was provided, would this patch already be deployed in iOS 8.1/8.1.3? 

And the iPad where I'm seeing this is model A1458 (4th generation), which I believe is is a 32-bit ARM device, but can you confirm (or tell me where I can)? http://support.apple.com/en-us/HT201471 isn't helpful on these details.
Comment 17 Benjamin Poulain 2015-02-23 11:28:38 PST
(In reply to comment #16)
> Thanks Benjamin — so, any recommendations as to how I can determine if the
> issue I see on the site in question is actually related to this bug or not?
> Based on the date this patch was provided, would this patch already be
> deployed in iOS 8.1/8.1.3? 

No, this fix did not make it to 8.1.x.

> And the iPad where I'm seeing this is model A1458 (4th generation), which I
> believe is is a 32-bit ARM device, but can you confirm (or tell me where I
> can)? http://support.apple.com/en-us/HT201471 isn't helpful on these details.

The 4th gen iPad has a A6X, it is a ARMv7s CPU, a custom 32 bits chip. This bug would affect it.

If you have access to a 64bits device, the easiest way to confirm would be try the website on 64 bits.

Other than that, it would be hard to confirm this bug is the problem you see. What is happening is the nth-child cache gets corrupted the first time the style is resolved. The only way to bypass the cache corruption is to disable the CSS JIT for every selectors that uses :nth-child().
Comment 18 rob 2015-02-25 07:38:53 PST
OK, it looks like this is what we're seeing then: I got a hold of a 64-bit device and cannot reproduce this issue there. Thanks for the guidance.