Bug 250814 - scrollTop and scrollLeft always 0 in DOMContentLoaded handler on a scrolled page
Summary: scrollTop and scrollLeft always 0 in DOMContentLoaded handler on a scrolled page
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2023-01-18 17:13 PST by Ahmad Saleem
Modified: 2023-09-24 11:04 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-01-18 17:13:28 PST
Hi Team,

While going through Blink's commit, I came across this nasty bug where the test case make Safari do continuous reload and cause issue. I think it is good to fix it right away.

Test Case with continuous reloading - https://jsfiddle.net/v960ryzt/

^ in Safari 16.2 & STP161, it will continuously reload scrollbar while it does not in Chrome Canary 111 and Firefox Nightly 111. It seems like that the navigation is stuck and page is refreshing.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/3ad81cf0b724a2a25ee5cc79a09541d9a971ae15

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/loader/FrameLoader.cpp#2739

I just wanted to raise bug so we can fix it.

Thanks!
Comment 1 Ahmad Saleem 2023-01-20 04:21:38 PST
I tried this PR - https://github.com/WebKit/WebKit/pull/8809

>> THIS TIMEOUT - scroll-position-restored-on-back-at-load-event.html

But I think this test is wrong because it is timing-out in EWS as well and I tried fixing based on my understanding but it didn't work.

As it is - it is failing in Chrome Canary 111 & Firefox Nightly 111 as well but the scrollbar repainting or refreshing is not as severe as Safari. They just refresh page.

____

As for taking updated copy of test case from source.chromium.org, you have to bring following in WebKit:

> gesture-util.js
> back.html [which is just <script>history.back()</script>]

^ Although, I think 'gesture-util.js' is not needed because I tried to look into one by one function or variable which one is from it in the test but couldn't find.


____

Appreciate if someone can give any direction or input to fix this test. Thanks!
Comment 2 Radar WebKit Bug Importer 2023-01-25 17:14:29 PST
<rdar://problem/104674675>
Comment 3 Ahmad Saleem 2023-02-19 15:50:39 PST
I manage to fix the last failing test but it is failing in iOS, which might be because we might have platform limitation.

Appreciate if someone can have a quick look and see if we need this - https://github.com/WebKit/WebKit/pull/8809

I can add platform specific exception in iOS unless except if it is genuine bug. I have seen various "scroll" tests being skipped in iOS.
Comment 4 Ahmad Saleem 2023-04-04 05:18:48 PDT
(In reply to Ahmad Saleem from comment #3)
> I manage to fix the last failing test but it is failing in iOS, which might
> be because we might have platform limitation.
> 
> Appreciate if someone can have a quick look and see if we need this -
> https://github.com/WebKit/WebKit/pull/8809
> 
> I can add platform specific exception in iOS unless except if it is genuine
> bug. I have seen various "scroll" tests being skipped in iOS.

I noticed that the didFirstLayout() code has special path for iOS platform, which was added as part of bug 125879.

I think we might have to skip these failing tests and since these are only failing tests on iOS platform and all other tests pass. So I think it is safe change.

@Simon - Any input?
Comment 5 Ahmad Saleem 2023-09-24 11:04:15 PDT
This also works:

if (isBackForwardLoadType(m_loadType) || isReload(m_loadType))