Bug 231563

Summary: iOS 17: History back sometimes scrolls to top of the page (with app banner)
Product: WebKit Reporter: Mads Erik Forberg <mads>
Component: ScrollingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Major CC: beidson, jotoffen, ok+bugzilla, simon.fraser, smoley, vetle, webkit-bug-importer, webkit
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: iPhone / iPad   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=267421
Attachments:
Description Flags
iOS15.4 buggy
none
Scroll bug on DT.no
none
iPadOS showing issue
none
iOS 15.4.1 NYT scroll bug when app banner is visiblle
none
iOS 15.14.1 VG scroll bug on app banner none

Description Mads Erik Forberg 2021-10-12 01:08:04 PDT
Steps to reproduce:
1. Go to a page (e.g. https://www.vg.no)
2. Scroll down a bit and click one of the links
3. Swipe back / navigate back to previous page
4. The page will sometimes scroll to the top of the page

See screen recording here: https://drive.google.com/file/d/1iCBs7JbVV5QalKktedhKxHyTT5K0vy_F/view?usp=sharing
Comment 1 Mads Erik Forberg 2021-10-18 10:38:24 PDT
Seems like more users and sites and experiencing this:


https://piunikaweb.com/2021/08/11/ios-15-beta-5-doesnt-fix-safari-forgets-scroll-position-on-previous-page/
Comment 2 Mads Erik Forberg 2021-10-18 11:02:09 PDT
(In reply to Mads Erik Forberg from comment #1)
> Seems like more users and sites and experiencing this:
> 
> 
> https://piunikaweb.com/2021/08/11/ios-15-beta-5-doesnt-fix-safari-forgets-
> scroll-position-on-previous-page/

The article says iOS 15 beta 4 and 5. But it’s still present here on iOS 15.1 latest public beta
Comment 3 Radar WebKit Bug Importer 2021-10-19 01:09:14 PDT
<rdar://problem/84405417>
Comment 4 Mads Erik Forberg 2021-10-20 00:03:21 PDT
I've created a small hack / fix for this behavior until the issue is fixed:

https://gist.github.com/forberg/183bc3e5cd3af3465e1830ff827443c0
Comment 5 Mads Erik Forberg 2022-01-27 00:39:37 PST
Any updates regarding this issue? Seems to be present in latest release and beta of iOS/Safari.
We see this issue on multiple sites.
Including:
https://www.bt.no
https.//www.aftenbladet.no
Comment 6 Mads Erik Forberg 2022-01-28 03:54:29 PST
We've posted a write up about the issue here:
https://aftenposten.substack.com/p/a-change-in-ios-15-is-negatively
Comment 7 Simon Fraser (smfr) 2022-01-28 12:08:18 PST
It looks like some of these sites now have the workaround? Is there a way I can test without the workaround?
Comment 8 Mads Erik Forberg 2022-01-28 13:22:45 PST
Thank you for the response, Simon.

Yes - that's correct. I've now added a flag via query string to disable it: https://www.vg.no/?ios-bug=true

I'm able to reproduce the bug on that URL.
Comment 9 Simon Fraser (smfr) 2022-01-28 13:34:13 PST
I believe this is fixed in the most recent iOS release (iOS / iPadOS 15.3/19D50). Please test and confirm.
Comment 10 Mads Erik Forberg 2022-01-28 14:20:29 PST
It seems like it's still present in iOS 15.4 Public Beta (19E5209h) when testing on my device (iPhone Xs) using this test URL: https://www.vg.no/?ios-bug=true
Comment 11 Mads Erik Forberg 2022-01-28 14:22:55 PST
Screen recording of testing (with iOS 15.4): 
https://drive.google.com/file/d/1Rqf0gqS7cgKDmKG0d7or3U5SL6Cqwood/view?usp=sharing
Comment 12 Mads Erik Forberg 2022-03-24 03:45:06 PDT
This is still present in latest iOS 15.4 - any updates here? :-)
Comment 13 Mads Erik Forberg 2022-03-24 03:52:32 PDT
The bug can easily be reproduced via this URL: https://www.vg.no/?ios-bug=true
Comment 14 Simon Fraser (smfr) 2022-03-24 11:34:01 PDT
I tried a number of times to reproduce this with that URL and was unable.
Comment 15 olekenneth 2022-03-25 04:38:39 PDT
Created attachment 455745 [details]
iOS15.4 buggy
Comment 16 olekenneth 2022-03-25 04:39:39 PDT
I'm using iPhone 12 with iOS 15.4 (19E241).
Comment 17 Mads Erik Forberg 2022-04-19 02:57:01 PDT
Created attachment 457873 [details]
Scroll bug on DT.no

Also buggy on the site www.dt.no
Comment 18 Mads Erik Forberg 2022-04-21 03:44:09 PDT
... and it's still present in latest public beta of iOS 15.5 (19F5057e).
Comment 19 Mads Erik Forberg 2022-04-21 03:55:38 PDT
While testing, I discovered the way that's the easiest to reproduce this:

1. Start a new tab session in Safari
2. Navigate to https://www.vg.no/?ios-bug=true (where the workaround is disabled)
3. Scroll down to a teaser / link a bit out of the first viewport.
4. Swipe / navigate back / use history back and the bug will trigger.

The clue seems to trigger more often when the browser / tab session is "fresh". I'm able to reproduce the bug on multiple devices, both physical and via emulation on BrowserStack.

I hope this will make it easier for you to reproduce the bug.
Comment 20 Mads Erik Forberg 2022-04-22 03:33:53 PDT
On the test page: https://www.vg.no/?ios-bug=true - I've added a `console.log({ scrollY });` in a `pageshow` event, which checks if the page is persisted, which then says that the Y position is `0`.

I've been able to reproduce this on both physical device and via BrowserStack.
Comment 21 Simon Fraser (smfr) 2022-04-22 10:47:07 PDT
I managed to reproduce this just once. The console.log() never fires in my experience.

It may be that page loading speed or behavior differs when loading it from a US IP address.
Comment 22 Simon Fraser (smfr) 2022-04-22 12:18:37 PDT
It's also possible this is related to the app banner. Do devices you reproduce this on also have the native app installed? Does removing the app change behavior?
Comment 23 olekenneth 2022-04-22 22:51:44 PDT
Did a test after removing the app and same result. But good suggestion. 

I see when I'm testing I'm almost always able to reproduce it. But I notice one thing. I'm swiping back while still scrolling or scrollbar are still visible on the page. Could this be relevant?
Comment 24 Simon Fraser (smfr) 2022-04-25 11:12:10 PDT
You're swiping back while still scrolling on the page you navigated to? So it's more like:
1. Tap the teaser link
2. On the page that lots, scroll a bit
3. Swipe back?
Comment 25 olekenneth 2022-04-27 01:04:05 PDT
Yes, but I did some tests now and made sure to stop scroll completely before swiping back and still had the problem. I'm taking to the top of the page right away. So probably not the issue
Comment 26 Johannes Andersen 2022-04-27 06:36:22 PDT
Created attachment 458441 [details]
iPadOS showing issue

iPad Pro (11-inch)
iPadOS: 15.5 (19F5062g)
Comment 27 Mads Erik Forberg 2022-04-28 02:46:26 PDT
*Might* be related; We see that the bug mostly happens if the browser navigates to the same origin and back. So if you click a teaser on `https://www.vg.no/?ios-bug=true` that goes to `https://www.vg.no/*` it seems to trigger.
Comment 28 Mads Erik Forberg 2022-05-10 00:07:27 PDT
Any updates regarding this issue? Seems like the bug triggers when navigating to same origin from the teasers.
Comment 29 Vetle Arneberg 2022-05-12 02:02:32 PDT
Created attachment 459212 [details]
iOS 15.4.1 NYT scroll bug when app banner is visiblle

This recording shows how meeting the app banner on either page (origin or destination) causes this scroll bug to happen on NY times, using iOS 15.14.1
Comment 30 Vetle Arneberg 2022-05-12 02:05:23 PDT
Created attachment 459213 [details]
iOS 15.14.1 VG scroll bug on app banner

To make sure what happened on NYT was consistent, I tested this on VG where the app banner is only shown in articles.

The bug occurred every time when the app banner was displayed in the article, however when I navigated to a page without the app banner, then the issue did not happen.
Comment 31 Jim Grisham 2022-07-13 18:05:02 PDT
This issue can be observed on this ~very~ Bugzilla page.

Example:
--------
In comment #4 above[1], there is a link.

This works as expected:
 1. click on that link
 2. scroll a little, or scroll a lot
 3. Ensure ~all~ inertial/momentum scrolling has stopped ~before~ navigating back (via button or swipe) to this page

This does not(*):
 1. click on link
 2. scroll a little, or scroll a lot
 3. Before the viewport has come to a complete stop, navigate back to this page
 Result: you will be returned to the top of the page (or to that comment, if the URL includes the `#c4` fragment element.

* it fails perhaps 90% of the time; the remainder may be attributable to human error, scrolling speed/acceleration, or something else entirely

Conjecture:
-----------
It seems that this might (only?) occur with **‘in-progress’ scrolling** immediately before the ‘back’ navigation. (Which would be much more likely to occur from stray events on touch devices than on mouse-based interfaces.)

Questions:
----------
1. Does that residual movement somehow ‘leak’ into the original page sometimes, so the ‘saved’ scroll position is not restored? ~or~
 - Is the scroll position indeed restored but then overridden immediately (i.e. the main thread is receiving the UI events directly, or via some framework, and in a well-meaning attempt to not let that residual motion affect the new content, ‘resetting’ the window scroll position to the top)?

2. Does horizontal scrolling also trigger this bug?

3. Is this definitely a ~direct~ WebKit bug, or could it actually be an interaction between WebKit and some underlying change between iOS 14 and iOS 15 framework event/concurrency/thread pool behaviors[2]?
 - Has this been tested in any other native apps that may use the same window controller as is used for Mobile Safari?
 - If WebKit is explicitly storing scroll position, it should be possible to adjust how it is restored. If it is ~not~, was it relying on some automatic behavior that is no longer available (or hidden behind an opaque abstraction layer)?

4. It seems any URL / server / network specific causes could be excluded by trying this on a webpage linking to a ‘data:’ URL (or other on-device source) while the device is on ‘Airplane Mode’, right? (e.g. Simple HTML, no JS, no CSS)

 - Jim

  [1]: https://bugs.webkit.org/show_bug.cgi?id=231563#c4
  [2]: e.g. similar to how Swift 5.7 changed the behavior of functions such as `doLongRunningWork()`[3]
  [3]: https://developer.apple.com/documentation/xcode/improving-app-responsiveness
Comment 32 Jim Grisham 2022-07-13 18:36:12 PDT
Quick update:

This can actually occur during both backwards ~and~ forwards navigation.

 - - - -

It appears (the? / at least one?) trigger may be if a ‘Pull-to-Refresh’ action was initiated immediately prior to the navigation (and thus is likely still in progress).

Due to touch gesture acceleration curves, this is quite easy to accomplish, especially on smaller screens. Incidentally, this is the same gesture often used to recall the toolbar (with its nav buttons) from the ‘collapsed’ state.

If the viewport is scrolling towards the bottom when the navigation occurs, the scroll position of the original page seems to be restored as expected. 

 - - - -

After the above discovery, a quick search surfaced a few (ostensibly) unrelated issues, such as [this one over at StackOverflow][1], which tends to lead me back to question #3 above.

Isolation test case: if ‘Pull-to-Refresh’ were disabled for a particular page using JavaScript[2], does the bug behavior still occur when navigating away from that page?

 - Jim


  [1]: https://stackoverflow.com/questions/69261011/disable-pull-to-refresh-in-ios-15-safari
  [2]: https://developer.apple.com/forums/thread/697360
Comment 33 Simon Fraser (smfr) 2022-08-04 14:32:39 PDT
(In reply to Jim Grisham from comment #32)

> It appears (the? / at least one?) trigger may be if a ‘Pull-to-Refresh’
> action was initiated immediately prior to the navigation (and thus is likely
> still in progress).


This helped me to get a reproducible testcase on news.ycombinator.com. I'm able to make progress now.
Comment 34 Simon Fraser (smfr) 2022-08-04 15:11:12 PDT
This happens when this comparison of two scales fails:

        if (WTF::areEssentiallyEqual<float>(contentZoomScale(self), _scaleToRestore))

When I was reproducing:
contentZoomScale 1.15044
_scaleToRestore 1.15
Comment 35 Simon Fraser (smfr) 2022-08-04 21:56:25 PDT
A little more detail here. I was reproducing this on new.ycombinator.com after using the Safari UI to set a 115% zoom scale. In this scenario, Safari calls [webView _setViewScale:1.15] in each -didCommitNavigation.

On Back navigation, we hit WebPage::restorePageState(), which has:
        float boundedScale = historyItem.scaleIsInitial() ? m_viewportConfiguration.initialScale() : historyItem.pageScaleFactor();

Here historyItem.scaleIsInitial() is true, and m_viewportConfiguration.initialScale() is 1.14999999, while historyItem.pageScaleFactor() is 1.15044248.

So the divergence happened earlier, and it's not just about float/double.
Comment 36 Simon Fraser (smfr) 2022-08-04 22:10:26 PDT
I confirmed that 1.15 turns into 1.15044 via the round-trip through UIScrollViews' zoomFactor.
Comment 37 Simon Fraser (smfr) 2022-08-04 22:40:01 PDT
There are also two code paths that call into scroll restoration logic. The first is when parsing finishes (this only hits when not going back to a page in the page cache, which is NOT the case on Hacker News):

  * frame #0: 0x00000003385fcfa3 WebKit`WebKit::WebPage::restorePageState(this=0x00007fdb5e80a408, historyItem=0x00007fdb6e82fa60) at WebPageIOS.mm:481:15
    frame #1: 0x00000003390bbc21 WebKit`WebKit::WebFrameLoaderClient::restoreViewState(this=0x00007fdb5e70b630) at WebFrameLoaderClient.cpp:1424:30
    frame #2: 0x000000037e3fd474 WebCore`WebCore::FrameLoader::HistoryController::restoreScrollPositionAndViewState(this=0x00007fdb5e70d7e0) at HistoryController.cpp:155:31
    frame #3: 0x000000037e3fd7d1 WebCore`WebCore::FrameLoader::didFirstLayout(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:2688:19
    frame #4: 0x000000037e60b64e WebCore`WebCore::FrameView::fireLayoutRelatedMilestonesIfNeeded(this=0x00007fdb5f80fa00) at FrameView.cpp:5523:26
    frame #5: 0x000000037e60cbd2 WebCore`WebCore::FrameView::performPostLayoutTasks(this=0x00007fdb5f80fa00) at FrameView.cpp:3538:9
    frame #6: 0x000000037e63ba0a WebCore`WebCore::FrameViewLayoutContext::runAsynchronousTasks(this=0x00007fdb5f80fb40) at FrameViewLayoutContext.cpp:320:12
    frame #7: 0x000000037e63cfe6 WebCore`WebCore::FrameViewLayoutContext::runOrScheduleAsynchronousTasks(this=0x00007fdb5f80fb40) at FrameViewLayoutContext.cpp:306:5
    frame #8: 0x000000037e63c773 WebCore`WebCore::FrameViewLayoutContext::performLayout(this=0x00007fdb5f80fb40) at FrameViewLayoutContext.cpp:282:9
    frame #9: 0x000000037e63bcb4 WebCore`WebCore::FrameViewLayoutContext::layout(this=0x00007fdb5f80fb40) at FrameViewLayoutContext.cpp:174:5
    frame #10: 0x000000037d84d73b WebCore`WebCore::Document::implicitClose(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/OpenSource/WebKitBuild/Debug-iphonesimulator/TestWebKitAPI.resources/simple-tall.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:3257:37
    frame #11: 0x000000037e3eca0b WebCore`WebCore::FrameLoader::checkCallImplicitClose(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:961:25
    frame #12: 0x000000037e3ec429 WebCore`WebCore::FrameLoader::checkCompleted(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:902:5
    frame #13: 0x000000037e3ea360 WebCore`WebCore::FrameLoader::finishedParsing(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:807:5
    frame #14: 0x000000037d86355b WebCore`WebCore::Document::finishedParsing(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/OpenSource/WebKitBuild/Debug-iphonesimulator/TestWebKitAPI.resources/simple-tall.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:6248:25
    frame #15: 0x000000037df48b18 WebCore`WebCore::HTMLConstructionSite::finishedParsing(this=0x00007fdb5f029c40) at HTMLConstructionSite.cpp:427:16
    frame #16: 0x000000037df8b490 WebCore`WebCore::HTMLTreeBuilder::finished(this=0x00007fdb5f029c00) at HTMLTreeBuilder.cpp:2862:12
    frame #17: 0x000000037df54d33 WebCore`WebCore::HTMLDocumentParser::end(this=0x00007fdb5f82c600) at HTMLDocumentParser.cpp:446:20

and the second is when loading finishes (or we go back to a cached page, I think):

  * frame #0: 0x00000003385fcfa3 WebKit`WebKit::WebPage::restorePageState(this=0x00007fdb5e80a408, historyItem=0x00007fdb6e82fa60) at WebPageIOS.mm:481:15
    frame #1: 0x00000003390bbc21 WebKit`WebKit::WebFrameLoaderClient::restoreViewState(this=0x00007fdb5e70b630) at WebFrameLoaderClient.cpp:1424:30
    frame #2: 0x000000037e3fd474 WebCore`WebCore::FrameLoader::HistoryController::restoreScrollPositionAndViewState(this=0x00007fdb5e70d7e0) at HistoryController.cpp:155:31
    frame #3: 0x000000037e3faed1 WebCore`WebCore::FrameLoader::checkLoadCompleteForThisFrame(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:2579:27
    frame #4: 0x000000037e3ec709 WebCore`WebCore::FrameLoader::checkLoadComplete(this=0x00007fdb5e70d5a0) at FrameLoader.cpp:2772:32
    frame #5: 0x000000037e3750ff WebCore`WebCore::DocumentLoader::finishedLoading(this=0x00007fdb2e80c600) at DocumentLoader.cpp:507:24

and the behavior can vary between them.
Comment 38 Simon Fraser (smfr) 2022-08-05 12:07:53 PDT
Pull request: https://github.com/WebKit/WebKit/pull/3052
Comment 39 EWS 2022-08-08 11:02:26 PDT
Committed 253221@main (95d1502e5e90): <https://commits.webkit.org/253221@main>

Reviewed commits have been landed. Closing PR #3052 and removing active labels.
Comment 40 Mads Erik Forberg 2022-11-22 04:11:38 PST
As mentioned on Slack, we're still seeing issues with scroll restoration when app is installed and "Open in App"-banner is present.

Steps to reproduce:
1. Install official VG-app: https://apps.apple.com/us/app/vg/id381071021
2. Goto https://www.vg.no/?ios-bug=true (where we have disabled the basic workaround)
3. Click one of the teasers going to www.vg.no origin *important*. Make sure it's opened in Safari, and not the VG app. (Tap to open context menu on teaser -> Open in Safari)
4. Navigate history back by swipin or arrows in the navigation bar.


The site scrolls to the top of the page every time.
Comment 41 Mads Erik Forberg 2023-08-24 05:26:51 PDT
Still an issue in iOS 17 public beta. Is there any hope of fixing the issue? It still seems present if the user visits a site, has that sites app installed and that site has an app-banner.
Comment 42 Simon Fraser (smfr) 2023-10-17 19:02:18 PDT
<rdar://117107620>
Comment 43 Simon Fraser (smfr) 2023-10-19 14:52:38 PDT
Looking at the issue triggered when a banner is present, things to bad in WebPage::restorePageState() in the cause where minimumLayoutSizeInScrollViewCoordinates differs, so we take the second branch. Here we try to restore using historyItem.unobscuredContentRect() but that gives us the wrong result.
Comment 44 Simon Fraser (smfr) 2023-10-19 16:10:05 PDT
This works correctly when we just use the scroll position because HistoryController::saveScrollPositionAndViewStateToItem() uses frameView->cachedScrollPosition(). There is no cachedUnobscuredContentRect() equivalent.
Comment 45 Simon Fraser (smfr) 2023-10-19 18:15:43 PDT
Pull request: https://github.com/WebKit/WebKit/pull/19324
Comment 46 EWS 2023-10-20 19:47:04 PDT
Committed 269611@main (2bafc1bcaa71): <https://commits.webkit.org/269611@main>

Reviewed commits have been landed. Closing PR #19324 and removing active labels.