RESOLVED FIXED 24477
REGRESSION (r41467): Page Down key scrolls two pages
https://bugs.webkit.org/show_bug.cgi?id=24477
Summary REGRESSION (r41467): Page Down key scrolls two pages
nobody
Reported 2009-03-09 20:00:16 PDT
hitting "Page Down" keyboard button broken (scrolls more than one "page", i.e. more than one screen distance) problem observed in r41521, not sure if that is first appearance of problem, but this bug appeared very recently.
Attachments
patch to fix double page down/page up bug (1.52 KB, patch)
2009-03-12 23:30 PDT, Rajiv Aaron Manglani
adele: review-
Alexey Proskuryakov
Comment 1 2009-03-10 09:02:40 PDT
Are you seeing this on all sites? I cannot reproduce the problem with r41550 (although I have a notebook, so it's Fn+Down, not PgDown for me).
nobody
Comment 2 2009-03-10 10:57:30 PDT
Yes, every website. The "Page Down" key is located on keyboard among Home/End/Delete/Page Up/Page Down/Insert buttons. (btw, "Page Up" scrolls the wrong distance, also.) Problem continues in r41545.
Alexey Proskuryakov
Comment 3 2009-03-10 11:39:29 PDT
Weird, it suddenly started to happen to me. Maybe I was testing an old version previously. Logging editing commands, I see that doCommandBySelector:"scrollPageDown:" is invoked twice for each PageDown.
Rajiv Aaron Manglani
Comment 4 2009-03-10 20:38:08 PDT
i'm seeing this in WebKit r41545 with safari 4 public beta installed: Version 4 Public Beta (5528.16, r41545), on a mac with an "extended keyboard" with actual page up/page down keys. the problem is not just with page down but also with page up.
Alexey Proskuryakov
Comment 5 2009-03-12 02:41:59 PDT
Rajiv Aaron Manglani
Comment 6 2009-03-12 23:02:15 PDT
this was broken in Changeset 41467 for trunk/WebKit/mac/WebView/WebHTMLView.mm in order to fix bug #24079. see http://trac.webkit.org/changeset/41467 building webkit r41660 with that change reversed fixes the problems described in this bug.
Rajiv Aaron Manglani
Comment 7 2009-03-12 23:30:29 PDT
Created attachment 28572 [details] patch to fix double page down/page up bug since the selector is passed to super, consider the event handled. code originally introduced in http://trac.webkit.org/changeset/39549
Adele Peterson
Comment 8 2009-03-12 23:35:01 PDT
Comment on attachment 28572 [details] patch to fix double page down/page up bug This will cause https://bugs.webkit.org/show_bug.cgi?id=23060 to show up again. To reproduce that bug, try running Mail.app with your patched WebKit. Focus a message in the preview pane, and you'll see that Home, End, Page Up, and Page Down don't work.
Adele Peterson
Comment 9 2009-03-13 09:07:26 PDT
Here's an experiment we should try. If we added the selectors for Page Up/Down and Home/End to the Editor command table in WebCore, would we still need eventHandled to be false here to get the Mail testcase to work?
Darin Adler
Comment 10 2009-03-17 16:23:52 PDT
I値l take this one.
Darin Adler
Comment 11 2009-03-17 17:06:41 PDT
Mark Rowe (bdash)
Comment 12 2009-03-17 17:24:52 PDT
*** Bug 24660 has been marked as a duplicate of this bug. ***
nobody
Comment 13 2009-03-19 05:11:19 PDT
This problem is not fixed. Some recent versions seemed OK. r41760 again scrolls too far. It seems to be not quite a full page extra, but it is scrolling the wrong distance.
Darin Adler
Comment 14 2009-03-19 07:39:38 PDT
The difference in distance scrolled is probably due to the fact that we now use WebCore's scrolling code instead of WebFrameView's. The old bug was absolutely critical, the new bug simply makes the Mac version match the Windows version. While I want to fix the new bug, it's *not* the same thing, so reopening this bug report isn't quite right.
Doug Harris
Comment 15 2009-03-19 08:01:02 PDT
(In reply to comment #13) > This problem is not fixed. > > Some recent versions seemed OK. > > r41760 again scrolls too far. According to comment #12, this was committed in r41793 -- which is after r41760. There has been no new nightly build since r41760 (I don't know why).
Darin Adler
Comment 16 2009-03-19 10:03:02 PDT
Please try again with a version newer than r41793.
nobody
Comment 17 2009-03-19 18:57:44 PDT
seems to be OK in r41844. Thanks!
Cameron Zwarich (cpst)
Comment 18 2009-03-20 02:17:01 PDT
*** Bug 24698 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.