24117 - [gfx scrollbar] Middle mouse button click should position scrollbar

Status: RESOLVED FIXED

(Core :: XUL, enhancement, P5)

Description

[Richard Zach]

On X Windows, you expect the scroll bar grabber to jump to the mouse position
when you click the middle mouse button.  This works in NS 4.7.

To reproduce:
1. Go to a longish page
2. Middle-click in the scrollbar area beneath the grabber

Actual result: Nothing happens
Expected result: Grabber should jump to where you clicked, so that middle of
grabber is under the mouse pointer.

Linux 2000.01.15.08

Comment 1

[Peter Trudelle]

I don't think this was ever made a requirement, which would make it an
unexpected feature for us.  assigning to evaughan as p5 for m20.  Setting
severity to enhancement, cc pavlov.

Comment 2

[Paul MacQuiddy]

bug 24196 is the same, assigned to blizzard, you can fight over who gets it.

Comment 3

[Christopher Blizzard (:blizzard)]

*** Bug 24196 has been marked as a duplicate of this bug. ***

Comment 4

[Tomi Leppikangas]

Now (20000210) when i middle click on gfx-scrollbar, it pastes clipbord
to browser. Should only do this on content area.

Would be nice to have same function than in NS 4.x. This works ok with native
(gtk) scrollbars.

Comment 5

[tenthumbs]

Middle-clicking works with both Motif and GTK scrollbars. Even xterms do
something with middle-clicks. Mozilla certainly should be compatible with
(almost) all the other Unix GUI apps.

Comment 6

[Paul MacQuiddy]

*IGNORE* - more massive spam, changing open XPToolkit bug's QA contact to
[e-mail]

Comment 7

[Paul MacQuiddy]

*IGNORE* - massive spam changing open XPToolkit bug's QA contact to
[e-mail]

Comment 8

[Peter Trudelle]

Mass move of all M20 bugs to M30.

Comment 9

[Akkana Peck]

Note: this would be a good project for someone who wants to see this behavior. 
It could probably be done nearly all in XBL (I hope!).  Adding a helpwanted
keyword (hope that's okay, Peter).

Comment 10

[Peter Trudelle]

Mass move of M20 bugs to M30

Comment 11

[Eric Vaughan]

In the future yes. But at the moment much of scrollbar behavior is still in C++ 
as XBL matures we are moving more and more out. We eventually want most of the 
widget to be entirely written in XBL and javascript. When that happens this will 
indeed be an easy thing to do. But I don't think its quite ripe yet.

Comment 12

[Davor Cubranic]

I've been unsuccessfully trying to figure out how the middle-button event gets
handled by the view instead of the scrollbar, and if anyone could give me some
pointers on where to look, I'd really appreciate it. As far as I can tell,
mozilla/layout/html/base/src/nsGfxScrollFrame.cpp is where the GFX scrollbars
should be created (the GFX ones only are exhibiting this behaviour -- turning
off the "gfx scrollbars" option in Debug preferences, thus using GTK ones, works
fine). But for the life of me, I couldn't track where they get registered for
events and where their event handlers should be. 

Comment 13

[Peter Trudelle]

Mass-moving all M20-M30 XPToolkit bugs to Future

Comment 14

[Davor Cubranic]

Attachment: Don't try to paste if middle-click was in a scrollbar

Comment 15

[Davor Cubranic]

These two patches (submitted separately as attachments) seem to fix the problem
for me. The one to layout/xul/base/src/nsSliderFrame.cpp moves the scrollbar
thumb to the location of the middle-click, while the the other, to
xpfe/browser/resources/content/navigator.js, checks whether a middle-click was
in a scrollbar before attempting to paste.

Comment 16

[Davor Cubranic]

Attachment: Move scrollbar's thumb to the location of the middle-click

Comment 17

[Tomi Leppikangas]

Patches were great. But you cant drag after middle click, so hacked
a little more to make it more like 4.x. Also put #ifdef XP_UNIX around
changes. Patch attached.

Comment 18

[Tomi Leppikangas]

Attachment: patch v2 for nsSliderFrame.cpp

Comment 19

[Davor Cubranic]

I found a case that's still problematic: trying to middle-click in a scrollbar
on a text field that's become long enough for a scrollbar to be added. (Such as
a Bugzilla report, for example. :-) What happens is that the thumb gets
positioned properly, but the paste is also executed. I haven't yet figured out
where, but it's not from xpfe/browser/resources/content/navigator.js -- I
checked, and the browserHandleMiddleClick function from there doesn't seem to
even get called.

Comment 20

[Tomi Leppikangas]

I think that textfield problem is more general bug with textfield; if you click
on textfield scrollbar's up/down/left/right arrow, you loose your selection,
or selection is changed. This doesnt happen on other scrollbars, just on
textfield.

Comment 21

[Eric Vaughan]

This looks good. I'm not sure we can check it in for beta2 unless it it deemed a 
stability or performance fix. You might want to talk to [e-mail] or 
[e-mail] and see what they think. But its a nice fix. We should get 
it into the build if we can.

Comment 22

[Akkana Peck]

Why ifdef XP_UNIX?  This sounds great for any platform that has a middle mouse
(thanks for doing it!)  If some non-Unix users don't want it, why not pref it
the same way all the other middle mouse stuff has been prefed?

Comment 23

[Davor Cubranic]

Good point about dragging, Tomo. I've never really paid too much attention to
it, so I didn't even think of that part when I started on the patch. I noticed
one more difference in its behaviour: the thumb should be placed on middle-down,
not up, and from there the dragging should go on exactly like with the left
button. The attached patch should do this, as well as clean up the code a bit.

The patch is against v2, I hope that's OK. Is that's how it's supposed to be
done, as opposed to doing it against what's currently in the CVS?

Comment 24

[Davor Cubranic]

Attachment: Patch v3 for nsSliderFrame.cpp -- fixes for middle-drag

Comment 25

[Davor Cubranic]

Attachment: The *real* patch v3 for nsSliderFrame.cpp -- pls disregard previous one

Comment 26

[Eric Vaughan]

Actually a diff against the current version in CVS would be the easiest to 
integrate.

Comment 27

[Tomi Leppikangas]

Davor, i think you get my patch wrong, my patch is against cvs, not
against your patch. I think my patch already did what that v3 patch does?

Try removing nsSliderFrame.cpp and 'cvs update nsSliderFrame.cpp' and then
apply my patch.

Comment 28

[Davor Cubranic]

My apologies for the confusion about applying patches. This is my first
contribution to Mozilla and I carelessly (not knowing better) applied Tomi's
patch over mine, instead of on a fresh file. Tomi is right, his patch already
did the same as my v3 -- with one exception: when middle-clicking on the thumb
slider, it should first position the middle of the thumb under the click, and
then start dragging from there (at least that's how Motif does it, and I think
it's a sensible thing to do). The attached patch (v4) should fix that.

Comment 29

[Davor Cubranic]

Attachment: Patch v4 for nsSliderFrame.cpp

Comment 30

[Tomi Leppikangas]

I changed those #ifdef XP_UNIX to pref "middlemouse.scrollbarPosition".
To get this working, put:
user_pref("middlemouse.scrollbarPosition", true);
to your prefs.js file.

When this goes to tree, i think modules/libpref/src/init/all.js should
read:
pref("middlemouse.scrollbarPosition", false);

and modules/libpref/src/unix/unix.js:
pref("middlemouse.scrollbarPosition", true);

Comment 31

[Tomi Leppikangas]

Attachment: patch v5 for nsSliderFrame.cpp and nsSliderFrame.h

Comment 32

[Eric Vaughan]

I just checked in this patch.

I patched 
nsSliderFrame.cpp
nsSliderFrame.h
unix.js
all.js
navigator.js

And put your names in as contributors. Thanks for all the great work.