Last Comment Bug 310380 - middlemouse autoscroll default-on for UNIX/Linux (X) environment
: middlemouse autoscroll default-on for UNIX/Linux (X) environment
Status: RESOLVED FIXED
: fixed1.8
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: unspecified
: x86 Linux
: -- normal with 2 votes (vote)
: ---
Assigned To: Hogarth
:
Mentors:
: 313119 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-28 17:46 PDT by Hogarth
Modified: 2008-07-31 04:22 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Adds pref to allow toggling of autoscroll, with defaults based on OS. (1.55 KB, patch)
2005-09-28 17:51 PDT, Hogarth
neil: superreview-
Details | Diff | Splinter Review
previous patch with requested modifications (1.34 KB, patch)
2005-10-16 03:47 PDT, Hogarth
csthomas: review+
neil: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Hogarth 2005-09-28 17:46:53 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050717 SeaMonkey/1.0a
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050717 SeaMonkey/1.0a

In the land of UNIX/Linux the middle mouse button is for pasting by default and
as such this feature is interfering and there's no easy workaround to set it
'right'.

Reproducible: Always

Steps to Reproduce:
1. Select text
2. Click to paste
3. Get autoscroll activated

Actual Results:  
Autoscroll was activated.

Expected Results:  
clipboard buffer should've been pasted
Comment 1 Hogarth 2005-09-28 17:51:50 PDT
Created attachment 197780 [details] [diff] [review]
Adds pref to allow toggling of autoscroll, with defaults based on OS.

I have patch thanks to CThos help. :)

It creates a pref called middlemouse.autoscroll and it sets it to false in UNIX
land and true otherwise. The all.js file is a tad messy in this respect so that
might ned careful eyeballing.

First patch to moz so please be gentle with the chainsaw. ;)
Comment 2 neil@parkwaycc.co.uk 2005-09-29 16:27:01 PDT
Could you explain why you don't find the existing middlemouse prefs suffice? By
default on Linux autoscroll is disabled because contentLoadURL takes priority;
even if you disable that then paste and scroll prefs apply where appropriate.
Comment 3 Hogarth 2005-09-29 18:13:49 PDT
(In reply to comment #2)
> Could you explain why you don't find the existing middlemouse prefs suffice? By
> default on Linux autoscroll is disabled because contentLoadURL takes priority;
> even if you disable that then paste and scroll prefs apply where appropriate.

My middlemouse prefs are as follows:

middlemouse.contentLoadURL: false
middlemouse.openNewWindow: true
middlemouse.paste: true
middlemouse.scrollbarPosition: true

and I still get the autoscroller. Even if I were to middle-click outside of a
paste field I do not want the autoscroller. This is why I've disabled
contentLoadURL. It was the height of annoyance and frustration to have the
browser suddenly go elsewhere (or in the latest case, suddenly lock the
functionality of the mouse and do 'weird' things to the display) because I
either missed the paste field or a link or accidentally clicked the middle mouse
button.

Right now, I have no way out what-so-ever. :/
Comment 4 Bruno 'Aqualon' Escherl 2005-10-02 09:00:46 PDT
I don't want autoscroll and I don't want to enable contentLoadURL under Windows,
since it's even more distracting. So the only workaround at the moment is to
default the return value of isAutoscrollBlocker(node) in contentAreaClick.js to
true at the beginning of the function.

Since Firefox even has an UI button for that, it would be the least to allow
changing that behaviour over a preference.
Comment 5 Frank Wein [:mcsmurf] 2005-10-02 16:52:39 PDT
I wonder if could not name this pref general.autoScroll like in Firefox? Why
create a newly named pref when there is already one which has the same purpose
and already appears in docs on the Internet.
Comment 6 Hogarth 2005-10-04 22:59:53 PDT
(In reply to comment #5)
> I wonder if could not name this pref general.autoScroll like in Firefox? Why
> create a newly named pref when there is already one which has the same purpose
> and already appears in docs on the Internet.

I don't use Firefox so I did not know what existed there. If the reviewers would
prefer that name then I'll remake the patch with general.autoScroll rather then
middlemouse.autoscroll as the pref name. (although the latter makes more sense
to me then the former :)
Comment 7 neil@parkwaycc.co.uk 2005-10-11 09:45:28 PDT
Comment on attachment 197780 [details] [diff] [review]
Adds pref to allow toggling of autoscroll, with defaults based on OS.

>+pref("middlemouse.autoscroll", true);
As mentioned this needs to be general.autoScroll and it also belongs in
browser-prefs.js instead.

>+  var prefService = Components.classes["@mozilla.org/preferences-service;1"];
>+  prefService = prefService.getService(Components.interfaces.nsIPrefService);
>+  var prefBranch = prefService.getBranch(null);
It looks like you copied an "inferior" version - the one in goPreferences is
superior.

>+  if (!prefBranch.getBoolPref("middlemouse.autoscroll"))
>+    return;
>+
>   if (gScrollingView || event.button != 1)
>     return;
Couldn't you combine these in one big condition?
Comment 8 Hogarth 2005-10-11 16:17:09 PDT
(In reply to comment #7)
> (From update of attachment 197780 [details] [diff] [review] [edit])
> >+pref("middlemouse.autoscroll", true);
> As mentioned this needs to be general.autoScroll and it also belongs in
> browser-prefs.js instead.

No problem. Will do.

> >+  var prefService = Components.classes["@mozilla.org/preferences-service;1"];
> >+  prefService = prefService.getService(Components.interfaces.nsIPrefService);
> >+  var prefBranch = prefService.getBranch(null);
> It looks like you copied an "inferior" version - the one in goPreferences is

:) The inferior version does seem to be used in a lot of places. :)

> superior.

So, something like this?

var pref =
Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);

if (!pref.getBoolPref("general.autoScroll"))
    return;

Looking closer it seems both the old and new method appear to be used
interchangeably.

> >+  if (!prefBranch.getBoolPref("middlemouse.autoscroll"))
> >+    return;
> >+
> >   if (gScrollingView || event.button != 1)
> >     return;
> Couldn't you combine these in one big condition?

I thought of that as I was writing it but decided to do the above because:

a. It looks a lot cleaner
b. Conceptually they are checking for different things
c. Other people elsewhere in the same file appeared to have a similar idea so
   it appeared to be the way the style flowed.

I'm not fussed though so if the merging of the two ifs is required then I will
do so. Getting this in is more important to me (esp since I wasted all day
yesterday thinking there was a bug in some php code but all along it was a bug
in my old version of seamonkey (opera and lynx both work fine)).

I'll work up a patch once the ifedness gets cleared up (ie merge or no).
Comment 9 Marko Macek 2005-10-15 09:41:33 PDT
I seem to have a bigger problem. This functionality is very annoying sometimes
(I keep pasting random pieces of text, for some reason).

Now in firefox 1.5 beta 2 the setting to disable this doesn't work.

Even enabling autoscroll which is a more useful feature for me doesn't help.
Comment 10 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-10-15 10:29:12 PDT
(In reply to comment #9)
> I seem to have a bigger problem. This functionality is very annoying sometimes
> (I keep pasting random pieces of text, for some reason).
> Now in firefox 1.5 beta 2 the setting to disable this doesn't work.
> Even enabling autoscroll which is a more useful feature for me doesn't help.

Firefox's autoscroll is entirely different from SeaMonkey's.  Please file a bug 
under the Firefox product.
Comment 11 neil@parkwaycc.co.uk 2005-10-15 16:43:48 PDT
(In reply to comment #8)
>So, something like this?
> 
>var pref =
Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
Yes, but that long line needs to be split just before the second "." to line up
on the next line with the first "." above it.

>I'll work up a patch once the ifedness gets cleared up (ie merge or no).
OK, then don't bother merging the ifs.
Comment 12 Hogarth 2005-10-16 03:47:33 PDT
Created attachment 199720 [details] [diff] [review]
previous patch with requested modifications

This is the same patch as before but with the following (requested) changes:

* middlemouse.autoscroll changed to general.autoScroll
* used different method of grabbing pref
* set pref in browser-prefs.js instead
Comment 13 neil@parkwaycc.co.uk 2005-10-16 04:38:20 PDT
Comment on attachment 199720 [details] [diff] [review]
previous patch with requested modifications

>+  var pref = Components.classes["@mozilla.org/preferences-service;1"]
>+                       .getService(Components.interfaces.nsIPrefBranch);
>+
>+  if (!pref.getBoolPref("general.autoScroll"))
>+    return;
>+
>   if (gScrollingView || event.button != 1)
>     return;
Although now that I look at it again I think I'd prefer if these two conditions
were exchanged.
Comment 14 Hogarth 2005-10-16 05:10:53 PDT
(In reply to comment #13)
> (From update of attachment 199720 [details] [diff] [review] [edit])
> >+  var pref = Components.classes["@mozilla.org/preferences-service;1"]
> >+                       .getService(Components.interfaces.nsIPrefBranch);
> >+
> >+  if (!pref.getBoolPref("general.autoScroll"))
> >+    return;
> >+
> >   if (gScrollingView || event.button != 1)
> >     return;
> Although now that I look at it again I think I'd prefer if these two conditions
> were exchanged.

*nod* Looking at them, the latter might come up more often and be the faster
check. If you don't want to swap the two ifs around yourself I can generate a
new patch tonight.
Comment 15 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-10-16 08:12:56 PDT
Comment on attachment 199720 [details] [diff] [review]
previous patch with requested modifications

No need for a new patch version to flip the if order.  Just have whoever checks
it in make that change.

r=cst

Please also request approval1.8rc1 so we can get this for SeaMonkey 1.0.
Comment 16 Hogarth 2005-10-16 15:46:46 PDT
(In reply to comment #15)
> (From update of attachment 199720 [details] [diff] [review] [edit])
> No need for a new patch version to flip the if order.  Just have whoever checks
> it in make that change.

Cool.

> Please also request approval1.8rc1 so we can get this for SeaMonkey 1.0.

Done. Thanks. :)
Comment 17 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-10-16 17:08:34 PDT
Checked in on trunk.
Comment 18 Elmar Ludwig 2005-10-20 06:27:45 PDT
*** Bug 313119 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.