Bug 18992

Summary:

Reset scroll position when loading new page

Product:

WebKit

Reporter:

Robin Dunn <robin>

Component:

WebKit wx

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

kevino

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
one liner for resetting the scrollbar	mrowe: review-
new patch to fix scrolling	kevino: review+

Robin Dunn

Reported 2008-05-10 21:44:22 PDT

In wxWebKit the scrollbar is not reset when loading a new page. This patch fixes that.

Attachments
one liner for resetting the scrollbar (456 bytes, patch) 2008-05-10 21:46 PDT, Robin Dunn	mrowe: review-	Details Formatted Diff Diff
new patch to fix scrolling (11.67 KB, patch) 2008-05-11 18:02 PDT, Robin Dunn	kevino: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Robin Dunn

Comment 1 2008-05-10 21:46:00 PDT

Created attachment 21060 [details] one liner for resetting the scrollbar

Mark Rowe (bdash)

Comment 2 2008-05-10 23:30:44 PDT

Comment on attachment 21060 [details] one liner for resetting the scrollbar windowObjectCleared is definitely not the right place to be doing this. It's likely there is some underlying issue that is leading to the incorrect behaviour you're seeing -- none of the other ports call setContentsPos from within WebKit, which suggests to me that doing so for wx is papering over an issue elsewhere.

Kevin Ollivier

Comment 3 2008-05-11 00:30:04 PDT

(In reply to comment #2) > (From update of attachment 21060 [details] [edit]) > windowObjectCleared is definitely not the right place to be doing this. It's > likely there is some underlying issue that is leading to the incorrect > behaviour you're seeing -- none of the other ports call setContentsPos from > within WebKit, which suggests to me that doing so for wx is papering over an > issue elsewhere. > Yeah, poking around some more, I think the real issue is that we don't handle FrameLoaderClientWx::transitionToCommittedForNewPage(), which deletes and re-creates the FrameView, and that's probably how all the other ports end up having their scroll positions reset.

Robin Dunn

Comment 4 2008-05-11 18:02:58 PDT

Created attachment 21075 [details] new patch to fix scrolling This patch does it by implementing transitionToCommittedNewPage() so the scroll positions are reset when a new page is loaded, and also maintained so that back and next restore the scroll positions as well. This also simplifies the logic for initializing and managing wxWebView.

Kevin Ollivier

Comment 5 2008-05-11 22:03:59 PDT

Comment on attachment 21075 [details] new patch to fix scrolling This does the trick! :-)

Kevin Ollivier

Comment 6 2008-05-11 22:10:18 PDT

landed in r33036, thanks! :)

Note You need to log in before you can comment on or make changes to this bug.