Last Comment Bug 782143 - The mousewheel preference pane is broken because the mousewheel preferences have all been changed in Bug 719320 (Implement DOM3 wheel event)
: The mousewheel preference pane is broken because the mousewheel preferences h...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.15
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on: 719320 782175
Blocks: 817979 823647
  Show dependency treegraph
 
Reported: 2012-08-12 12:42 PDT by Philip Chee
Modified: 2012-12-20 12:50 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed


Attachments
Modify mousewheel preference pane to reflect the changes from bug #718320. (v1) (30.14 KB, patch)
2012-08-15 02:32 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Splinter Review
Modify mousewheel preference pane to reflect the changes from bug #718320. (v2) (23.43 KB, patch)
2012-08-15 21:21 PDT, Edmund Wong (:ewong)
philip.chee: feedback+
Details | Diff | Splinter Review
Modify mousewheel preferences pane per changes from bug 719320. (v3) (24.22 KB, patch)
2012-08-16 07:51 PDT, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Splinter Review
Modify mousewheel preference pane per changes from bug 719320. (v4) (22.30 KB, patch)
2012-08-17 19:53 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Splinter Review
Modify mousewheel preferences pane per changes from bug 718320. (v5) (22.30 KB, patch)
2012-08-18 07:35 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Splinter Review
Possible patch (30.13 KB, patch)
2012-08-22 13:41 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
Details | Diff | Splinter Review
Addressed review comments (31.42 KB, patch)
2012-08-29 12:40 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
iann_bugzilla: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Philip Chee 2012-08-12 12:42:58 PDT
The old preferences are gone. We'll need to redo our pref-mousewheel prefpane to use the new preferences ASAP.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2012-08-12 23:39:24 PDT
Thank you for your report.

The prefs are documented here:
https://wiki.mozilla.org/Gecko:Mouse_Wheel_Scrolling#Mozilla_17_.28Firefox_17.29_or_later

Feel free to ask me if you have some questions.
Comment 2 Edmund Wong (:ewong) 2012-08-15 02:32:22 PDT
Created attachment 652044 [details] [diff] [review]
Modify mousewheel preference pane to reflect the changes from bug #718320. (v1)
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2012-08-15 02:50:09 PDT
Note that Meta key is NOT available on Windows. And Win key is NOT available on Mac. So, it may be better if they can be hidden on the platforms.
Comment 4 neil@parkwaycc.co.uk 2012-08-15 14:36:53 PDT
The delta multipliers actually affect all of the options (except do nothing, of course), which lets us simplify the pane a bit. (In fact you could start off by writing the basic pane without the delta modifiers for now, and add the modifiers in a followup bug.) We could also stick to the current four modifier key choices for now.
Comment 5 Edmund Wong (:ewong) 2012-08-15 21:21:11 PDT
Created attachment 652319 [details] [diff] [review]
Modify mousewheel preference pane to reflect the changes from bug #718320. (v2)
Comment 6 Philip Chee 2012-08-16 03:49:53 PDT
Comment on attachment 652319 [details] [diff] [review]
Modify mousewheel preference pane to reflect the changes from bug #718320. (v2)

Looks good. While you're at it, since you're rewriting the whole pref pane, can you remove the trailing white space in pref-mouswheel.xul as well?

Note for Nakano-san: for some reason holding the ALT key down disables the scroll wheel completely. No matter what which option I choose (Scroll document, move back/forwards through history, or zoom page; nothing happens. Is this a mouse driver problem? CTRL and SHIFT, and WIN keys work fine.
Comment 7 Edmund Wong (:ewong) 2012-08-16 07:51:26 PDT
Created attachment 652454 [details] [diff] [review]
Modify mousewheel preferences pane per changes from bug 719320. (v3)
Comment 8 Ian Neal 2012-08-16 14:44:15 PDT
Comment on attachment 652454 [details] [diff] [review]
Modify mousewheel preferences pane per changes from bug 719320. (v3)

># HG changeset patch
># Parent 7387d34b4335997895cd12065de394357dcc8842
># User Edmund Wong <ewong@pw-wspx.org>
>Bug 782143 - Modify mousewheel preferences pane per changes from bug 718320.
Nit: bug 719320

>+++ b/suite/common/pref/pref-mousewheel.js
> function Startup()
> {
Worth adding a comment that this function will be used in a follow-up bug?

>+++ b/suite/common/pref/pref-mousewheel.xul

>     <preferences id="mousewheel_preferences">
>+      <preference id="mousewheel.default.action"
>+                  name="mousewheel.default.action"
>                   type="int"/>
>+      <preference id="mousewheel.default.delta_multiplier_x"
>+                  name="mousewheel.default.delta_multiplier_x"
>                   type="int"/>
The delta preferences aren't used at the moment, so don't add them here, add them in the followup when they do get used.

>+      <preference id="mousewheel.with_meta.action"
>+                  name="mousewheel.with_meta.action"
>+                  type="int"/>

>+      <preference id="mousewheel.with_win.action"
>+                  name="mousewheel.with_win.action"
>+                  type="int"/>
The meta and win preferences aren't used in this patch, so don't add them here, add them in the followup.


>     <tabbox class="spaced">
>       <tabs>
>         <tab label="&usingJustTheWheel.label;"/>
>         <tab label="&usingWheelAndAlt.label;"/>
>         <tab label="&usingWheelAndCtrl.label;"/>
>         <tab label="&usingWheelAndShft.label;"/>
Should we be having accesskeys on the tabs, or is that too complicated?

r- for the moment as I would like to review the new patch.
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2012-08-16 17:33:02 PDT
(In reply to Philip Chee from comment #6)
> Note for Nakano-san: for some reason holding the ALT key down disables the
> scroll wheel completely. No matter what which option I choose (Scroll
> document, move back/forwards through history, or zoom page; nothing happens.
> Is this a mouse driver problem? CTRL and SHIFT, and WIN keys work fine.

I guess so. I don't have the problem on my environments. What are you using your mouse/touchpad and the version of the its utils?
Comment 10 Philip Chee 2012-08-17 04:18:05 PDT
> I guess so. I don't have the problem on my environments. What are you using your
> mouse/touchpad and the version of the its utils?
Synaptics touchpad V7.4 Driver is v15.0.8.1 2010-03-10
Comment 11 Edmund Wong (:ewong) 2012-08-17 19:53:03 PDT
Created attachment 653012 [details] [diff] [review]
Modify mousewheel preference pane per changes from bug 719320. (v4)
Comment 12 Philip Chee 2012-08-17 22:18:45 PDT
Also see:
Bug 782175 Support slower scroll settings with mousewheel.*.delta_multiplier_*
Comment 13 Ian Neal 2012-08-18 00:54:52 PDT
Comment on attachment 653012 [details] [diff] [review]
Modify mousewheel preference pane per changes from bug 719320. (v4)

>+<!ENTITY usingWheelAndShft.accesskey     "h">

h is used by Help
Comment 14 Edmund Wong (:ewong) 2012-08-18 07:35:55 PDT
Created attachment 653057 [details] [diff] [review]
Modify mousewheel preferences pane per changes from bug 718320. (v5)

Changed Shift's tab accesskey from h to f.
Comment 15 neil@parkwaycc.co.uk 2012-08-22 13:41:40 PDT
Created attachment 654357 [details] [diff] [review]
Possible patch

I thought I'd take a stab at the delta preferences.

This isn't based on ewong's patch so you'll notice a few minor differences, in particular I left the groupboxes in, I didn't add access keys for tabs, and I made up different names for the new entities.
Comment 16 Ian Neal 2012-08-27 14:15:00 PDT
Comment on attachment 654357 [details] [diff] [review]
Possible patch

>+++ b/suite/locales/en-US/chrome/common/pref/pref-mousewheel.dtd

>+<!ENTITY mouseWheelAction.label          "Mouse wheel action">
>+<!ENTITY doNothing.label                 "Do Nothing">
Should this be "Do nothing" instead?
>+<!ENTITY doNothing.accesskey             "D">
>+<!ENTITY scrollDocument.label            "Scroll the document">
>+<!ENTITY scrollDocument.accesskey        "S">
> <!ENTITY history.label                   "Move back and forward in the browsing history">
> <!ENTITY history.accesskey               "M">
> <!ENTITY zoom.label                      "Zoom the page in or out">
> <!ENTITY zoom.accesskey                  "Z">
>+<!ENTITY mouseWheelSpeed.label           "Mouse wheel speed">
>+<!ENTITY verticalSpeed.label             "Vertical:">
>+<!ENTITY verticalSpeed.accesskey         "V">
>+<!ENTITY verticalReverse.label           "Reverse direction">
>+<!ENTITY verticalReverse.accesskey       "R">
>+<!ENTITY horizontalSpeed.label           "Horizontal:">
>+<!ENTITY horizontalSpeed.accesskey       "n">
Is "o" available?
>+<!ENTITY horizontalReverse.label         "Reverse direction">
>+<!ENTITY horizontalReverse.accesskey     "c">
Wouldn't "e" be better?

Is there much point having the "Mouse wheel speed" settings enabled when the action is set to "Do Nothing"/"Do nothing"?

r=me with those addressed/answered.
Comment 17 Ian Neal 2012-08-27 14:39:36 PDT
Comment on attachment 653057 [details] [diff] [review]
Modify mousewheel preferences pane per changes from bug 718320. (v5)

Cancelling request in favour of Neil's patch
Comment 18 neil@parkwaycc.co.uk 2012-08-29 12:08:37 PDT
(In reply to Ian Neal from comment #16)
> (From update of attachment 654357 [details] [diff] [review])
> >+<!ENTITY mouseWheelAction.label          "Mouse wheel action">
> >+<!ENTITY doNothing.label                 "Do Nothing">
> Should this be "Do nothing" instead?
Yes.

> >+<!ENTITY horizontalSpeed.label           "Horizontal:">
> >+<!ENTITY horizontalSpeed.accesskey       "n">
> Is "o" available?
> >+<!ENTITY horizontalReverse.label         "Reverse direction">
> >+<!ENTITY horizontalReverse.accesskey     "c">
> Wouldn't "e" be better?
I thought we weren't supposed to use vowels. (But c is particularly dire, I know.)
Comment 19 neil@parkwaycc.co.uk 2012-08-29 12:40:57 PDT
Created attachment 656556 [details] [diff] [review]
Addressed review comments

The "Do nothing" disabling was tricky because of the checkboxes. Then I realised that I'd forgotten about locked preferences anyway, so I fixed that too.
Comment 20 Philip Chee 2012-09-05 02:31:00 PDT
Pushed comm-central changeset 86111781add1
Comment 21 Philip Chee 2012-09-05 02:39:12 PDT
Comment on attachment 656556 [details] [diff] [review]
Addressed review comments

[Approval Request Comment]
Regression caused by (bug #): Bug 719320
User impact if declined: Mousewheel Preference pane totally broken.
Testing completed (on m-c, etc.): comm-central
Risk to taking this patch (and alternatives if risky): none. The base patch landed on Mozilla17 we just missed the train so we need to land on comm-aurora A.S.A.P.
String changes made by this patch: ** Significant changes to pref-mousewheel.dtd **
Comment 22 Ian Neal 2012-09-05 03:09:06 PDT
Comment on attachment 656556 [details] [diff] [review]
Addressed review comments

a=me with suitable notices to the relevant newsgroups.
Comment 23 Philip Chee 2012-09-06 02:20:37 PDT
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/eee7e7a90121

> a=me with suitable notices to the relevant newsgroups.
Sent message to m.d.l10n.
https://groups.google.com/group/mozilla.dev.l10n/browse_thread/thread/b42ada9330f362be
Comment 24 chAlx 2012-12-04 00:05:38 PST
Thanks to :masayuki for some pref cheats (Bug 801101):

> If you set delta_multiplier less than 100000 and a native wheel event's scroll
> amount is over one page, then, the actual scroll amount is limilted to one page.

So there are 3 ways to use delta_multiplier_* values (actually percents):

1-999 to scroll faster/slower;
~10000-99999 to scroll exactly one page up/down;
100000+ to scroll full document to top/bottom.

Not sure about the last one (home/end option), but pageup/pagedown options are very useful and can not be easily reproduced with keyboard (you need to wait for keypress autorepeat delay or to repeat tapping PgUp/PgDn rapidly). And there were an option to scroll one page in the old preferences dialog.

Now there is no such option (regression?) and the percentage fields are limited to input only [+-]1-999 values. It even can't print a large value (>999) already set manually.

I think the preferences dialog should be able to setup page (and may be document) scrolling via some self-explanatory options. And for sure it must output correct values without truncation.
Comment 25 neil@parkwaycc.co.uk 2012-12-04 00:37:31 PST
(In reply to chAlx from comment #24)
> Now there is no such option (regression?) and the percentage fields are
> limited to input only [+-]1-999 values. It even can't print a large value
> (>999) already set manually.
When the dialog was written I had no idea that people would want to enter in a huge value, although given the existence of bug 801101 this does of course become absolutely necessary, however you will need to file a new bug to track this (please make it depend on both bugs).

Does anyone know which Gecko (Firefox) version bug 801101 landed in?
Comment 26 Jens Hatlak (:InvisibleSmiley) 2012-12-04 00:43:10 PST
(In reply to neil@parkwaycc.co.uk from comment #25)
> Does anyone know which Gecko (Firefox) version bug 801101 landed in?

TM, which was set upon commiting, reads mozilla19, i.e. Gecko/FF 19, SM 2.16.
Comment 27 neil@parkwaycc.co.uk 2012-12-04 01:18:14 PST
(In reply to Jens Hatlak from comment #26)
> (In reply to neil@parkwaycc.co.uk from comment #25)
> > Does anyone know which Gecko (Firefox) version bug 801101 landed in?
> TM, which was set upon commiting
You can see I'm not up to speed on Bugzilla workflow ;-)

> mozilla19, i.e. SM 2.16.
Phew, so we've still got some time to address this change then.
Comment 28 chAlx 2012-12-04 02:25:44 PST
(In reply to neil@parkwaycc.co.uk from comment #25)
> I had no idea that people would want to enter in a huge value

BTW 999% is just 10x multiplying. It is too small for some circumstances (e.g. mouse driver gives one line scrolling and display prints 500 lines of small text).

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