Bug 136904

Summary: As of iOS 8.0 (12A365), all Mobify.js sites unexpectedly scroll down the page after load. Sometimes this happens on first load, and happens almost 100% of the time after navigating to another page on the website by clicking a link.
Product: WebKit Reporter: Shawn Jansepar <shawnjan>
Component: Page LoadingAssignee: Tim Horton <thorton>
Status: NEW ---    
Severity: Critical CC: benjamin, hussain, justin, mike.klemarewski, mikek, shawnjan, steve.calvert, stewart, thorton, webkit-bug-importer, yoav, zalan
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: Other   
Attachments:
Description Flags
Minimal test case
none
Correct iOS 8.0 scroll bug minimal test case.
none
Patch none

Description Shawn Jansepar 2014-09-17 18:25:07 PDT
Created attachment 238279 [details]
Minimal test case

A minimal test case has been attached and can be found in ios8_bug.html.

Steps to Reproduce:
On an iOS device:

1. Open attached io8_bug.html on an iOS8 device or iOS8 Simulator.
2. Notice that the initial rendering of the page is scrolled down.
3. Refresh to repeat.

or

1. Go to any of the sites listed in the Affected Sites list below and browse around.

Expected Results:
The initial rendering of the page should not be scrolled down.

Actual Results:
The initial rendering of the page is scrolled down.

Version:
iOS 8.0 [12A365]

Notes:
The minimal test case is attached. The minimal test case shows how the page is being scrolled after dynamically attaching the meta viewport tag. The reason this is related to mobify.js is because it uses a technique we call "capturing" - which does the following to the page:

1. Inserts a plaintext tag to capture the contents of the page in order to prevent resources from loading.
2. Creates an interstitial document that we use to query against in an effort to create a new HTML page for that specific route.
3. Creates the new page by rendering the mobile template chosen by the router using dust.js.
4. Renders that new page to the DOM by using document.open/write.

Since the document string is being written later on, and that document string almost always has the meta viewport tag in it, we end up running into the same issue exhibited by the minimal test case. We believe whatever issue plaguing the test case is the same issue we are running into with websites running Mobify.js.

Configuration:
iPhone 5S (iOS8), and iOS8 Simulator Xcode 6.0.1

Sites affected:

AnnTaylor.com
BeyondTheRack.com
Bosch-Home.com
BT.com
CallawayGolf.com
Coastal.com
Ideeli.com
Loft.com
Lululemon.com
www.menshealth.com
Neff.de
Siemens-Home.com
MyStarbucksIdea.force.com
Techvibes.com
Thermador.com
Threadless.com
TommyBahama.com
TriaBeauty.com
VirginWines.co.uk
Wired.com
WomensHealthMag.com
WSJWine.com
AICPA.org
ArchitecturalDigest.com
BET.com
BurlingtonCoatFactory.com
CTShirts.com
Chewy.com
Christies.com
CoastHotels.com
Crocs.co.uk
Epicurious.com
TravelNow.com
css-tricks.com
Foxhead.com
Garmin.com
GQ.com
iOffer.com
Ivivva.com
LHW.com
Outrigger.com
Patheos.com
Perfumania.com
SharperImage.com
StellaDot.com
Style.com
Superdry.com
TeenVogue.com
TheKnot.com
Theory.com
Comment 1 Radar WebKit Bug Importer 2014-09-18 10:01:00 PDT
<rdar://problem/18381757>
Comment 2 Benjamin Poulain 2014-09-25 16:50:57 PDT
I just tried the minimal test case on iPhone 5 and iPhone 6 and I could not reproduce the bug. What am I missing?

Note that the use case you describe seems to go against all the guidelines for efficient page loading.
Comment 3 Justin Vaillancourt 2014-09-25 17:32:07 PDT
(In reply to comment #2)
> I just tried the minimal test case on iPhone 5 and iPhone 6 and I could not reproduce the bug. What am I missing?
> 
> Note that the use case you describe seems to go against all the guidelines for efficient page loading.

Thanks for taking a look at this!

Unfortunately the attached test case was saved using `File -> Save Page As` and already includes the injected meta viewport as a consequence. Sorry about that. Here is a link to the same, but correct test case:

https://dl.dropboxusercontent.com/u/76629/ios8_bug.html
Comment 4 Justin Vaillancourt 2014-09-25 17:33:28 PDT
Created attachment 238687 [details]
Correct iOS 8.0 scroll bug minimal test case.

Also attached here.
Comment 5 Benjamin Poulain 2014-09-25 18:57:21 PDT
Ok, I can reproduce with this test case.
Comment 6 Shawn Jansepar 2014-09-29 13:31:40 PDT
Hi Benjamin, great that you were able to reproduce! That was about 4 days ago - has any movement been made on fixing the issue?
Comment 7 Benjamin Poulain 2014-09-29 13:54:43 PDT
(In reply to comment #6)
> Hi Benjamin, great that you were able to reproduce! That was about 4 days ago - has any movement been made on fixing the issue?

No, I am working on CSS at the moment.

From the information I have, this seems to be low priority. Adding the viewport meta tag after the first layout is considered bad practice because it creates so much overhead.
Comment 8 Shawn Jansepar 2014-09-29 14:25:24 PDT
I would agree that the way we dynamically load the meta tag is non-standard, but I don't think priority of tickets should be defined by whether or not they are "considered bad practice", it should be defined by the number of sites this is effecting, and how badly it is effecting them.

Currently, there are over 1000 sites using our framework - many of which are some very notable brands. You can see them in the list above.

As an aside, I would love to have a chat with you outside of this ticket about how our technology works and why we've taken the approach we've taken, in order to see if you think there is a better approach. It may seem like bad practice, but it's the best we've come up with based on the current limitations of the web :). 

At the end of the day, we rely on our technique, and the issue that has arisen in iOS8 is a regression.
Comment 9 Benjamin Poulain 2014-09-29 15:13:53 PDT
(In reply to comment #8)
> I would agree that the way we dynamically load the meta tag is non-standard, but I don't think priority of tickets should be defined by whether or not they are "considered bad practice", it should be defined by the number of sites this is effecting, and how badly it is effecting them.
> 
> Currently, there are over 1000 sites using our framework - many of which are some very notable brands. You can see them in the list above.

Certainly, the technical aspect is only a part of it.

So far I have received a total of 2 reports about this particular bug. As you can imagine, typical high priority bugs get hundreds of radars. I also did not receive any feedback on this during the 4 months of beta which is usually a strong sign this is not important.

I'll probably look at this bug in the coming weeks to understand what is going on, but from the data I have at this very moment, this is not a "drop everything and fix this" situation. I have other bug reports that seems to have higher value for users.

> As an aside, I would love to have a chat with you outside of this ticket about how our technology works and why we've taken the approach we've taken, in order to see if you think there is a better approach. It may seem like bad practice, but it's the best we've come up with based on the current limitations of the web :).

Yep, please email me. Best case we can extend the engine to accommodate your use case, worst case I fix this bug :)

> At the end of the day, we rely on our technique, and the issue that has arisen in iOS8 is a regression.

Yep, that is an unfortunate bug in iOS 8, and it needs to be fixed.
Comment 10 Shawn Jansepar 2014-10-02 01:18:53 PDT
(In reply to comment #9)
> So far I have received a total of 2 reports about this particular bug. As you can imagine, typical high priority bugs get hundreds of radars. I also did not receive any feedback on this during the 4 months of beta which is usually a strong sign this is not important.
> 
> I'll probably look at this bug in the coming weeks to understand what is going on, but from the data I have at this very moment, this is not a "drop everything and fix this" situation. I have other bug reports that seems to have higher value for users.

Yep fair enough. One thing I'd point out is that many developers using our framework have come talked to us about the issue, and as well, we've reached out to them, so they may not have felt the need to file the bug themselves. But yesh, I would agree that scrolling down the page is not a "drop everything and fix", but I also wouldn't consider it low priority either :) It's a strange experience for the user.

> Yep, please email me. Best case we can extend the engine to accommodate your use case, worst case I fix this bug :)

Will do! Looking forward to the dialog :)
Comment 11 Yoav Weiss 2014-10-03 05:14:45 PDT
Hi Benjamin,

Do you think that the bug is in code that's in the public repo? Is there something I can do to help resolve the issue quicker?

I've tried to reproduce the issue with the latest WebKit trunc, while enabling as much IOS code around viewport handling as I can, but didn't have much luck with that.
Comment 12 Benjamin Poulain 2014-10-04 11:41:15 PDT
(In reply to comment #11)
> Do you think that the bug is in code that's in the public repo? Is there something I can do to help resolve the issue quicker?
> 
> I've tried to reproduce the issue with the latest WebKit trunc, while enabling as much IOS code around viewport handling as I can, but didn't have much luck with that.

It is very likely in the public repository. The other option is a bug in UIKit.

I don't think you'll be able to reproduce by enabling the viewport on OS X, there are too many differences. You may be able to find the issue by code inspection of viewport update code.
Comment 13 Benjamin Poulain 2015-01-23 17:00:30 PST
Created attachment 245263 [details]
Patch
Comment 14 Shawn Jansepar 2015-09-11 10:20:55 PDT
Hey Benjamin, any chance this patch could drop for iOS9 or an early update? Our fix for the issue specifically targets iOS8, as we thought this issue would be fixed for iOS9. Please let me know so I can plan accordingly! Thx :)
Comment 15 Benjamin Poulain 2015-09-11 15:40:43 PDT
(In reply to comment #14)
> Hey Benjamin, any chance this patch could drop for iOS9 or an early update?
> Our fix for the issue specifically targets iOS8, as we thought this issue
> would be fixed for iOS9. Please let me know so I can plan accordingly! Thx :)

Adding Zalan.

Zalan finished the patch and it is fixed in WebKit trunk. BUT, IIRC, the fix had to be removed from iOS 9 because it caused problems.

Zalan, do you remember what happened to the top-edge snapping on viewport update?
Comment 16 zalan 2015-09-17 09:19:31 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > Hey Benjamin, any chance this patch could drop for iOS9 or an early update?
> > Our fix for the issue specifically targets iOS8, as we thought this issue
> > would be fixed for iOS9. Please let me know so I can plan accordingly! Thx :)
> 
> Adding Zalan.
> 
> Zalan finished the patch and it is fixed in WebKit trunk. BUT, IIRC, the fix
> had to be removed from iOS 9 because it caused problems.
> 
> Zalan, do you remember what happened to the top-edge snapping on viewport
> update?
The proposed patch caused regression and we rolled it out.
Comment 17 Shawn Jansepar 2015-09-17 09:56:55 PDT
Are there any plans to address the regression and get the patch in?
Comment 18 zalan 2015-09-17 11:50:43 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > Hey Benjamin, any chance this patch could drop for iOS9 or an early update?
> > Our fix for the issue specifically targets iOS8, as we thought this issue
> > would be fixed for iOS9. Please let me know so I can plan accordingly! Thx :)
> 
> Adding Zalan.
Adding Tim.