Bug 16424 - Enable auto-scroll in page by mouse middle click
Summary: Enable auto-scroll in page by mouse middle click
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P4 Enhancement
Assignee: Maxime Britto
URL:
Keywords: InRadar
Depends on:
Blocks: 19971
  Show dependency treegraph
 
Reported: 2007-12-13 11:18 PST by Louise
Modified: 2008-07-10 09:17 PDT (History)
3 users (show)

See Also:


Attachments
Add the feature without the icon yet (9.10 KB, patch)
2008-06-19 13:16 PDT, Maxime Britto
hyatt: review-
Details | Formatted Diff | Diff
patch attempt (26.36 KB, patch)
2008-07-02 18:53 PDT, Maxime Britto
no flags Details | Formatted Diff | Diff
patch (26.95 KB, patch)
2008-07-03 12:02 PDT, Maxime Britto
no flags Details | Formatted Diff | Diff
new patch (32.84 KB, patch)
2008-07-08 11:24 PDT, Maxime Britto
adele: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Louise 2007-12-13 11:18:49 PST
Is there a reason for not having this feature?
Since Firefox and Opera supports it, seems reasonablt for webkit to have it too.

I couldn't find any bug concerning this problem so I'm going to file one myself.
Please tell me if this is not an appropriate place to talk about this.
thanks.
Comment 1 Adam Roben (:aroben) 2007-12-13 11:23:42 PST
<rdar://5603777>
Comment 2 Robert Blaut 2008-02-18 14:01:19 PST
There is no reason to keeping enhancement requests in an unconfirmed state.
Comment 3 Maxime Britto 2008-06-19 13:16:08 PDT
Created attachment 21848 [details]
Add the feature without the icon yet

This patch adds the requested feature. I also need to work on the icon which should be displayed at the original middle click location (and stay still while scrolling) and will be issued in a future patch.
Comment 4 Dave Hyatt 2008-06-19 14:00:42 PDT
Comment on attachment 21848 [details]
Add the feature without the icon yet

This patch contains tabs.  Can you re-attach a tab-free version?
Comment 5 Dave Hyatt 2008-06-19 14:12:35 PDT
The pan scroll shouldn't trigger if you can't scroll the clicked area.

Firefox pan scrolls only documents, and does not pan scroll overflow:auto sections.  This honestly looks like a bug to me.  Your code is doing the same thing, but I think overflow:auto sections should pan scroll....

You might want to test what IE does with overflow sections on Windows when it pan scrolls.

Comment 6 Adam Roben (:aroben) 2008-06-19 14:22:23 PDT
Comment on attachment 21848 [details]
Add the feature without the icon yet

+	else if (mouseEvent.button() == MiddleButton && !mev.isOverLink()) {
+		startPanScroll();
+		invalidateClick();
+        return false;

I don't think !mev.isOverLink() is a good enough check here. You probably wouldn't want the pan to occur when you middle-click on a button.

In general, we may not want the pan to occur if event.preventDefault() is called. We should test in Firefox and IE to see if calling event.preventDefault() on a middle-click event prevents the pan from occurring. We should match their behavior in this regard.
Comment 7 Maxime Britto 2008-07-02 18:53:14 PDT
Created attachment 22057 [details]
patch attempt

Works only on windows.
Icon added.
Comment 8 Maxime Britto 2008-07-03 09:55:49 PDT
Comment on attachment 22057 [details]
patch attempt

I noticed some cosmetic changes so don't review this one, I'm preparing a new one.
Comment 9 Maxime Britto 2008-07-03 12:02:46 PDT
Created attachment 22068 [details]
patch

Made some functional and cosmetic corrections.
Comment 10 Maxime Britto 2008-07-08 11:24:40 PDT
Created attachment 22159 [details]
new patch

This version fix 2 problems I found with the previous one :
- It was possible to trigger multiple panScroll on frameset (one on each frame)
- It was possible to trigger the panScroll event when the object was not scrollable
Comment 11 Maxime Britto 2008-07-09 17:31:49 PDT
Fixed in r35083
Comment 12 Robert Blaut 2008-07-09 22:26:18 PDT
Why the patch is not reviewed?
Comment 13 Maxime Britto 2008-07-09 22:33:43 PDT
It has been reviewed by email this afternoon by Adele Peterson. She will mark it reviewed tomorrow on bugzilla I guess.
Comment 14 Adele Peterson 2008-07-10 09:17:58 PDT
Comment on attachment 22159 [details]
new patch

I reviewed this yesterday.