FrameLoader calls gotoAnchor() in finishedParsing(), which causes WebKit to scroll to an anchor after html has loaded, but before images load and scripts run. By the time elements have loaded, the original scrolling may no longer be the actual location of the anchor. Scrolling to the anchor before all elements have loaded makes sense, but ideally we would ensure that the anchor continues to be displayed, even as its position relative to the top of the document might change. http://code.google.com/p/chromium/issues/detail?id=619 appears to be an example of the image case of this issue.
Created attachment 30745 [details] patch
Comment on attachment 30745 [details] patch Yeah this is really close. The one thing I think you are missing here is that all scrolls should turn off the lock and not just user scrolls. For example if the page focuses a text field after the anchor jump, and we scroll to expose that text field, you can't then jump back to the anchor. If a page sets body.scrollTop back to 0, you can't jump back to the anchor after that, etc. Basically you need to turn off the lock if any scrolling happens AT ALL (other than the gotoAnchor scroll obviously).
Created attachment 30786 [details] patch2 Programmatic scroll now releases the anchor lock. Note that navigating to an anchor is a type of programmatic scroll, so when gotoAnchor() gets called, the anchor lock gets released in FrameView, then gotoAnchor() sets the anchor lock again before it returns.
Comment on attachment 30786 [details] patch2 No need to check if (lockedToAnchor()) in setWasScrolledByUser. Everything else looks good. r=me.
Created attachment 30789 [details] patch3 Removed the spurious if() and fixed a tab that was off.
Created attachment 30795 [details] patch4
(In reply to comment #6) > Created an attachment (id=30795) [review] > patch4 > Added a null check for m_frame->view() in FrameLoader::completed(), was intermittently crashing a few tests.
Doh, you forgot to mark it for review :) Nobody will ever find it! :)
Comment on attachment 30795 [details] patch4 Please don't forget about better layout tests. We need pretty, pretty layout tests.
Will land.
Fixed in r44311.
Please verify that the expected result landed in http://trac.webkit.org/changeset/44318 is correct. The attached patch is missing the expected result.
I'm going to reopen this bug to handle the additional layout tests that I need to add.
Comment on attachment 30795 [details] patch4 This patch has already been landed. Removing from commit queue.
Comment on attachment 30786 [details] patch2 I'm told if I remove the r+ from this obsolete patch, this will be removed from the commit queue.
This likely caused bug 27137.