RESOLVED FIXED 25570
WebKit incorrectly fires scroll events during the history.{back,forward,go} call when navigating to a reference fragment
https://bugs.webkit.org/show_bug.cgi?id=25570
Summary WebKit incorrectly fires scroll events during the history.{back,forward,go} c...
Darin Fisher (:fishd, Google)
Reported 2009-05-05 10:37:27 PDT
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.
Attachments
demo (746 bytes, text/html)
2009-05-05 10:38 PDT, Darin Fisher (:fishd, Google)
no flags
v1 patch (3.14 KB, patch)
2009-05-05 14:47 PDT, Darin Fisher (:fishd, Google)
no flags
v2 patch: this time with layout test changes (10.58 KB, patch)
2009-05-05 16:58 PDT, Darin Fisher (:fishd, Google)
darin: review+
Darin Fisher (:fishd, Google)
Comment 1 2009-05-05 10:38:14 PDT
Darin Fisher (:fishd, Google)
Comment 2 2009-05-05 10:47:33 PDT
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)
Comment 3 2009-05-05 14:28:27 PDT
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)
Comment 4 2009-05-05 14:47:16 PDT
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 5 2009-05-05 14:48:56 PDT
Comment on attachment 30034 [details] v1 patch Seems OK to me. Anders?
Darin Fisher (:fishd, Google)
Comment 6 2009-05-05 16:58:18 PDT
Created attachment 30041 [details] v2 patch: this time with layout test changes
Darin Fisher (:fishd, Google)
Comment 7 2009-05-05 22:00:35 PDT
Note You need to log in before you can comment on or make changes to this bug.