Bug 25570 – WebKit incorrectly fires scroll events during the history.{back,forward,go} call when navigating to a reference fragment

• Component: Page Loading
• Status: RESOLVED FIXED

[Darin Fisher (:fishd, Google)]

WebKit incorrectly fires scroll events during the history.{back,forward,go} call when navigating to a reference fragment

This is subtle.  None of the other browsers behave this way.  They always dispatch history.{back,forward,go} asynchronously.  I found this bug because Chrome also happens to behave like IE and FF.  Safari seems to be the only browser that behaves differently, so I think it is probably best that it changes to match the others.

See attached demo.

[Darin Fisher (:fishd, Google)]

Created attachment 30024 [details]
demo

[Darin Fisher (:fishd, Google)]

The code in FrameLoader::scheduleHistoryNavigation is what I am suggesting should perhaps change.  If the URL we are at and the URL we are navigating to differ only in reference fragment, then the code calls goBackOrForward directly instead of calling scheduleRedirection.  This results in the navigation occurring synchronously, which leads to scroll events being dispatched during the history.{back,forward,go} call.  That in turn means that JavaScript is getting re-entered, which may be quite unexpected by some web applications.

[Darin Fisher (:fishd, Google)]

It looks like this synchronous call to goBackOrForward was introduced by andersca in r14669: http://trac.webkit.org/changeset/14669/trunk/WebCore/page/Frame.cpp

That was done as part of the fix for bug 6309.

[Darin Fisher (:fishd, Google)]

Created attachment 30034 [details]
v1 patch

This patch is incomplete since I will need to correct several layout tests to account for this change, but I wanted to post this for review to get feedback on this approach.  Thanks!

[Darin Adler]

Comment on attachment 30034 [details]
v1 patch

Seems OK to me. Anders?

[Darin Fisher (:fishd, Google)]

Created attachment 30041 [details]
v2 patch: this time with layout test changes

[Darin Fisher (:fishd, Google)]

Landed as:  http://trac.webkit.org/changeset/43274