Scroll restoration breaks when document changes height during hash change navigation |
|||||
Issue description
Chrome Version : 59.0.3071.86
OS Version: OS X 10.12.4
URLs (if applicable) :
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
Safari 5:
Firefox 4.x:
IE 7/8/9:
What steps will reproduce the problem?
1. Load this file in the browser:
https://github.com/Lindi/Playground/blob/master/chrome-scroll-bug.html
2. Click anywhere in the body. This will add a hash fragment to window.location and double the number of div elements in the document body.
3. Scroll down to the bottom of the page.
4. Hit browser back.
What is the expected result?
The document scroll position should have been restored to the top of the page.
What happens instead of that?
The document scroll position is at the bottom of the page.
Please provide any additional information below. Attach a screenshot if
possible.
UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36
,
Jun 19 2017
Able to reproduce this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12.4 using chrome latest stable #59.0.3071.104. Bisect Information: -------------------- Good build: 58.0.2994.0 Bad Build : 58.0.2996.0 You are probably looking for a change made after 446830 (known good), but no later than 446831 (first known bad). Change Log URL: https://chromium.googlesource.com/chromium/src/+log/ebf15845426ed332bd1e2570c5a8488ad7e075ff..36b47e73e35bcea80ed02f6da9f8b3652545726c From the above change suspecting the below change Review-Url: https://codereview.chromium.org/2653673006 japhet@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Thanks!
,
Jun 21 2017
Thanks for the repro! onpopstate is forcing a scroll, after which we check whether to save that scroll state for history use. As of https://codereview.chromium.org/2653673006, this is incorrectly clobbering the scroll state that we're supposed to be restoring right after onpopstate finishes. I've got a patch in progress.
,
Jul 12 2017
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64015d479a86f06c19cac283a6e6214b38d1db10 commit 64015d479a86f06c19cac283a6e6214b38d1db10 Author: japhet <japhet@chromium.org> Date: Wed Jul 19 01:23:00 2017 Changing scroll and view state in onpopstate shouldn't overwrite back/forward state restore FrameLoader::SaveScrollState attempts to avoid clobbering state that will be used for a back/forward restore by checking the committed DocumentLoader::LoadType(). As of https://codereview.chromium.org/2653673006, same-document navigations don't modify that load type, so a same-document back-forward navigation can get its scroll state clobbered by creative use of an onpopstate event handler. Store the needed state on the stack, so that even if onpopstate does trigger a SaveScrollState(), we can still restore. This also pulls scroll and view state out into a helper class of HistoryItem. This object is nullptr until state is saved for the first time, so we don't need a bool to track whether the state is default or real. BUG= 734276 TEST=fast/history/change-viewport-height-in-onpopstate.html Review-Url: https://codereview.chromium.org/2949073002 Cr-Commit-Position: refs/heads/master@{#487706} [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/content/renderer/history_serialization.cc [add] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/LayoutTests/fast/history/change-viewport-height-in-onpopstate-expected.txt [add] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/LayoutTests/fast/history/change-viewport-height-in-onpopstate.html [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/exported/WebHistoryItem.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/exported/WebViewTest.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/frame/VisualViewportTest.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/loader/DocumentLoader.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/loader/FrameLoader.h [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/loader/HistoryItem.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/core/loader/HistoryItem.h [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/Source/web/tests/WebFrameTest.cpp [modify] https://crrev.com/64015d479a86f06c19cac283a6e6214b38d1db10/third_party/WebKit/public/web/WebHistoryItem.h
,
Jul 19 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ranjitkan@chromium.org
, Jun 19 2017