[Shift + Mouse-Scroll-Wheel] Does NOT Scroll Horizontally

Bug #1228250 reported by Lonnie Lee Best
28
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

In Chromium, I can scroll horizontally using my mouse-wheel by holding down the shift key while scrolling the mouse wheel.

Firefox is missing this very convenient page-navigation short-cut.

Use Case:

As my eyes age, I find myself always scaling up the web pages I read (by holding down crtl and scrolling my mouse wheel).

Doing this, often makes the page exceed the width of my monitor (hiding the right-side of the text I want to read) and produces a horizontal scroll bar at the bottom of the page.

At this point, since I've already used ctrl-scroll-mouse-wheel to magnify the page, it would be wonderful if I could use shift-scroll-mouse-wheel to horizontally-scroll the magnified page and therefore center the (previously cropped) text that I am wanting to read.

ProblemType: Bug
DistroRelease: Ubuntu 13.04
Package: firefox 24.0+build1-0ubuntu0.13.04.1
ProcVersionSignature: Ubuntu 3.8.0-30.44-generic 3.8.13.6
Uname: Linux 3.8.0-30-generic x86_64
NonfreeKernelModules: wl
AddonCompatCheckDisabled: False
ApportVersion: 2.9.2-0ubuntu8.3
Architecture: amd64
AudioDevicesInUse:
 USER PID ACCESS COMMAND
 /dev/snd/controlC0: lonnie 2161 F.... pulseaudio
 /dev/snd/pcmC0D0p: lonnie 2161 F...m pulseaudio
BrokenPermissions: sessionstore.bak (0o600, wrong owner)
BuildID: 20130911155223
Channel: Unavailable
Date: Fri Sep 20 11:02:07 2013
ForcedLayersAccel: False
IfupdownConfig:
 # interfaces(5) file used by ifup(8) and ifdown(8)
 auto lo
 iface lo inet loopback
InstallationDate: Installed on 2013-09-06 (14 days ago)
InstallationMedia: Ubuntu 13.04 "Raring Ringtail" - Release amd64 (20130424)
IpRoute:
 default via 192.168.24.1 dev eth0 proto static
 192.168.24.0/24 dev eth0 proto kernel scope link src 192.168.24.198 metric 1
MarkForUpload: True
PrefSources:
 prefs.js
 [Profile]/extensions/{b9db16a4-6edc-47ec-a1f4-b86292ed211d}/defaults/preferences/prefs-dwhelper.js
ProcEnviron:
 TERM=xterm
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
Profiles: Profile0 (Default) - LastVersion=24.0/20130911155223 (In use)
RelatedPackageVersions:
 google-talkplugin 4.5.3.0-1
 icedtea-7-plugin 1.3.2-1ubuntu1.1
 totem-mozilla 3.6.3-0ubuntu6
 rhythmbox-mozilla 2.98-0ubuntu5
RunningIncompatibleAddons: False
SourcePackage: firefox
UpgradeStatus: No upgrade log present (probably fresh install)
dmi.bios.date: 04/14/2011
dmi.bios.vendor: Dell Inc.
dmi.bios.version: A09
dmi.board.name: 0P792H
dmi.board.vendor: Dell Inc.
dmi.board.version: A09
dmi.chassis.type: 8
dmi.chassis.vendor: Dell Inc.
dmi.chassis.version: A09
dmi.modalias: dmi:bvnDellInc.:bvrA09:bd04/14/2011:svnDellInc.:pnStudio1737:pvrA09:rvnDellInc.:rn0P792H:rvrA09:cvnDellInc.:ct8:cvrA09:
dmi.product.name: Studio 1737
dmi.product.version: A09
dmi.sys.vendor: Dell Inc.

Revision history for this message
In , Hunters (hunters) wrote :

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0rc1)
Gecko/20020417
BuildID: 2002041711

Under Preferences->Advanced->Mouse Wheel, I'd like to see an option for mouse
wheel + modifier key to scroll the current page horizontally instead of
vertically (when such scrolling is possible).

Reproducible: Always
Steps to Reproduce:
Not currently a feature.

Revision history for this message
In , Bugzilla-tf (bugzilla-tf) wrote :

see also bug 58589

Revision history for this message
In , Jag-mozilla (jag-mozilla) wrote :

-> bryner

Revision history for this message
In , Skewermz (skewermz) wrote :

Note that the traditional defaults are:

Mouse wheel: Scroll vertically
Ctrl+wheel: Zoom
Shift+wheel: Pan (scroll horizontally)

Most of the programs that don't follow this tradition either pan when the mouse
is hovering over the horizontal scrollbar (bug 71464) or shamefully don't pan at
all.

Revision history for this message
In , Bugzilla-tf (bugzilla-tf) wrote :

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

Revision history for this message
In , Danielwang (danielwang) wrote :

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

Revision history for this message
In , Llwang (llwang) wrote :

I think that the OS should be All instead of just Windows 2000.

Revision history for this message
In , Vdvo (vdvo) wrote :

Indeed.

Revision history for this message
In , Hachiro75 (hachiro75) wrote :

Created attachment 140146
patch (only tested with gtk)

Hi.
This patch adds two options `scroll the document by [N] chars.' and `scroll a
page left or a page right.' to mousewheel's pref-panel.

Tha range of integer value of mousewheel.with*key.action is extended to 5.
4=scroll horizontally.
5=scroll page horizontally.

The result makes mousewheel's pref-panel more verbose
but may help single wheel mouse users.

Revision history for this message
In , Hunters (hunters) wrote :

Great! Any chance for a binary patch for Win32? :)

Revision history for this message
In , Hachiro75 (hachiro75) wrote :

Created attachment 140298
XUL (workaround)

(In reply to comment #9)
> Great! Any chance for a binary patch for Win32? :)

Thanks.

Though the best way is building Mozilla with patch,
here is yet another implementation by XUL.
This also implements Bug 71464 imperfectly.

This will be XPInstalled as add-on by simply opening it in Mozilla.
Only works with window, frame and message-pane in Mozilla but none of other
widgets like texbox although original patch works with almost every scrollable
widget.

If you want to change modifier key, amount of scroll or direction,
edit mozilla's chrome/whscroll/content/whscrollOverlay.xul manualy.
The value of mousewheel.with[your modifier]key.action must be 0 or 1 when using
as horizontal scroll trigger.

Uninstallation is done manualy delete the directory named `whscroll' and remove
descriptions about whscroll in chrome.rdf and those lines in some
overlayinfo/*/content/*.rdf files in Mozilla's chrome directory.

Revision history for this message
In , Bwucke+bug (bwucke+bug) wrote :

Existing workaround: Extension of MozGest
http://optimoz.mozdev.org/gestures/index.html
"wheel rocker" - hold the wheel depressed (or middle button) while scrolling
scrolls horizontally. But still I'd like to see this implemented as well.

Revision history for this message
In , pmow (pmow) wrote :

I fully agree that there should be GUI options to change horizontal mouse wheel
scrolling. But should they not just mirror the ones for vertical scrolling under
Preferences->Advanced->Mouse Wheel and utilizes the currently hidden preferences
mousewheel.horizscroll.with*key.* (that have bad defaults, see bug 231718)?

That would make much more sense to me than introducing new values for the
vertical/normal scrolling prefs.

Revision history for this message
In , pmow (pmow) wrote :

Created attachment 163247
UI prefs for mousewheel.horizscroll.* prefs

OK, this is my first try. I split each tab in the prefs panel Advanced->Mouse
Wheel into two group boxes, the upper for vertical scrolling, the lower for the
horizontal options. I also added a short paragraph to the respective help page.

Seems to work fine here on OS/2 with the current trunk, all changes I make in
the GUI panel are reflected in about:config and vice versa.

Revision history for this message
In , pmow (pmow) wrote :

Comment on attachment 163247
UI prefs for mousewheel.horizscroll.* prefs

Actually, this is probably better reviewed by the XPFE owners, although bryner
seems to be the expert on mousewheel scrolling...

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Iann-bugzilla (iann-bugzilla) wrote :

I think the change to help needs some work - it is just tagged on at the end
rather than being incorporated into the existing text. For example:

The line "The Mouse Wheel preferences panel allows you to control how the mouse
wheel on your mouse (in between your mouse buttons) is used in Mozilla:"
probably needs to make reference to the fact the user's mouse might have
multiple wheels or one that the direction can be adjusted. I only have a
standard wheel mouse so I'm not sure if there is a standard place for these
extra features to be on an advanced wheel mouse - if there is one, maybe that
needs to be mentioned as it is for the standard wheel mouse.

Perhaps - "The Mouse Wheel preferences panel allows you to control how the
vertical mouse wheel (in between your mouse buttons) and the horizontal mouse
wheel (<location of wheel> - if you have one) on your mouse are used in Mozilla:"

Revision history for this message
In , pmow (pmow) wrote :

Well, I thought that this would be explained somewhere in the mouse driver or
even OS help. But you are right, at least partly. And if I ever get review
comments for my patch I will certainly take your comments into account before it
gets checked in.

Revision history for this message
In , Jag-mozilla (jag-mozilla) wrote :

Comment on attachment 163247
UI prefs for mousewheel.horizscroll.* prefs

>Index: xpfe/components/prefwindow/resources/locale/en-US/pref-mousewheel.dtd
>===================================================================

> <!ENTITY scrollPgUpPgDn.label "Scroll a page up or a page down">
> <!ENTITY scrollPgUpPgDn.accesskey "p">
>+<!ENTITY scrollPgLtPgRt.label "Scroll a page left or a page right">
>+<!ENTITY scrollPgLtPgRt.accesskey "p">

Double definition of "p" as an accesskey.

>Index: extensions/help/resources/locale/en-US/cs_nav_prefs_advanced.xhtml
>===================================================================
> <p><strong>Note</strong>: Each modifier key can be assigned to a different
> function.</p>
> </li>
>+ <li>For each key the upper box titled "Vertical scrolling" allows you to
>+ assign a function to the normal operation of the wheel. If your mouse
>+ allows you to switch the wheel into horizontal mode or has two wheels
>+ you can use the lower box titled "Horizontal scrolling" to make similar
>+ adjustments as for the vertical mode.</li>

</li> should be on its own line as per the previous entry.

Other than that it looks good to me (though I haven't closely examinated your
copy&paste skills).

Go ahead and discuss better wording for the help file.

Revision history for this message
In , Neil-httl (neil-httl) wrote :

Comment on attachment 163247
UI prefs for mousewheel.horizscroll.* prefs

The horizontal radios will need completely separate accesskeys to the vertical
radios. They may currently only work on the last tab but that's a separate bug.
While you're there, make sure none of them (old or new) are 'h' because that's
reserved for help.

I noticed you reindented enableField, please don't include whitespace fixes on
otherwise unchanged code blocks.

You don't need ids on your new <vbox>es, please remove them.

Finally I don't think that the help is sufficiently helpful.

Revision history for this message
In , pmow (pmow) wrote :

Great, thanks for the reviews! Those are all easily changed, although the
accesskeys still only work in the last tab (the one with shift), but from neil's
comment I understand that as a known problem.

The wording of the help text is much harder, before I post everything again as a
full patch, here is my suggestion:

   The Mouse Wheel preferences panel allows you to control how the mouse wheel
   on your mouse (in between your mouse buttons) is used in Mozilla.
   Modern mice may have two wheels or a button that can be used to switch the
   scroll direction of the wheel. The behaviour for the vertical wheel function
   is set in the upper panel <strong>Vertical scrolling</strong> while the
   horizontal mode is controlled by the lower panel <strong>Horizontal
   scrolling</strong>.

1st big item:
   [... in the small items add "characters" in brackets after lines
        and left/right after up/down ...]
2nd big item:
   If your mouse does not have a mode for horizontal scrolling, any setting
   in the lower panel <strong>Horizontal scrolling</strong> will be ignored.

One could add another item about mice with back/forward buttons but I think that
goes to far here, and I don't have a mouse like that so I can only guess from
bug 64485 what should be happening.

Revision history for this message
In , pmow (pmow) wrote :

Steven Hunter writes me:

> > 2nd big item:
> > If your mouse does not have a mode for horizontal scrolling, any
> > setting in the lower panel <strong>Horizontal scrolling</strong>
> > will be ignored.
>
> Maybe I'm misunderstanding something here, but this statement doesn't
> jive with the intent of the "bug" in question.
>
> The point of the bug was to request that the Mouse Wheel pref panel list
> "Scroll the document horizontally" as a choice for the wheel action. Your
> comment seems to imply supporting mice with secondary wheels only for
> such scrolling.

I think you are right that I may have misunderstood the original intent of the
bug when I first found it. Since then I never read the old comments again. It's
a bit stupid because noone objected two months ago or at least when I added my
ideas and the first draft of my patch.
What's the best procedure now? I guess I need to open a new bug and attach the
new patch there... Objections?

Revision history for this message
In , Jag-mozilla (jag-mozilla) wrote :

Yeah, that would probably be the best way to go. Wow, I completely missed the
original request, which I think would be really nice to have (having run into
this desire myself on many occassions).

Revision history for this message
In , pmow (pmow) wrote :

OK, none of the other horizontal mouse wheel bugs really applied, so I created
bug 274179 and attached the patch there. Sorry again, I also change back this
bug's title, as the required patch obviously requires some backend changes in
addition to the UI.

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Sugar-waffle (sugar-waffle) wrote :

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

Revision history for this message
In , Mattsch (mattsch) wrote :

I don't know if this should be included in this enhancement or I should create a
new one but it would be nice for the people who don't have a horizontal
scrolling mouse to have horizontal scrolling capabilities with the mouse. If
anyone has ever used Textpad, you might know what I'm talking about. In this
program when a horizontal scrollbar exists, it is possible to press and hold the
control key and use the vertical scroll on the mouse to scroll left and right.
So would it be possible to hold the ctrl key + up scroll to scroll left and ctrl
key + down to scroll right? I could envision just including the same option
Scroll a page left or right under the vertical scrolling area as well.

Revision history for this message
In , Hunters (hunters) wrote :

(In reply to comment #30)

Uh, if you'll re-read the description, that's already the goal of this bug.

Revision history for this message
In , Mattsch (mattsch) wrote :

I was just looking into the new options in the latest nightly build and the
newest options under mouse wheel. I suppose left/right scrolling under vertical
scrolling will be a later patch?

Revision history for this message
In , Chpe (chpe) wrote :

Created attachment 185037
backend-only patch

Let's proceed step-by-step. Here's a patch implementing the backend support for
scrolling the other direction with the mouse wheel, which I need for embedding
(Epiphany).
It adds wheel actions 4 and 5, which scroll by numlines resp. by pagesize in
the other direction (i.e. horizontal for wheel, vertical for horiz wheel).

Revision history for this message
In , Dtownsend+bugmail (dtownsend+bugmail) wrote :

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

Revision history for this message
In , 935c (935c) wrote :

Supposedly my bug 303537 is a duplicate of this bug.

I fail to see why because :
(a) This appears to be a bug restricted to the Microsoft Windows platform
(b) The Macintosh version of Mozilla doesn't even have
"Preferences->Advanced->Mouse Wheel"

Revision history for this message
In , Jo-hermans (jo-hermans) wrote :

(In reply to comment #35)
> (b) The Macintosh version of Mozilla doesn't even have
> "Preferences->Advanced->Mouse Wheel"

I thought you were using Firefox ?

38 comments hidden view all 137 comments
Revision history for this message
Lonnie Lee Best (launchpad-startport) wrote :
Revision history for this message
Lonnie Lee Best (launchpad-startport) wrote :

Also, most mouse-scroll-wheels (these days) have the ability for you to click your mouse-wheel.

If you press down on your scroll wheel, while scrolling, this too should allow you to scroll horizontally in Firefox.

Revision history for this message
Lonnie Lee Best (launchpad-startport) wrote :

If the grease-monkey firefox addon installed, I found this user-script as a work around:
http://userscripts.org/scripts/show/168193

Changed in firefox:
importance: Unknown → Wishlist
status: Unknown → Confirmed
Changed in firefox:
status: Confirmed → In Progress
Changed in firefox (Ubuntu):
status: New → Confirmed
Changed in firefox:
status: In Progress → Confirmed
94 comments hidden view all 137 comments
Revision history for this message
In , Thomas Mitterfellner (tomm-sbox) wrote :

I know it's not qt, but qt has this notion of QWheelEvent::DefaultDeltasPerStep – maybe gtk has that too?

Anyway, I recently implemented a fix for a problem in the linux KDE application gwenview, where, when you used CTRL+Touchpad_scroll to zoom in/out, it was horribly sensitive (see here for bug/patch https://bugs.kde.org/show_bug.cgi?id=378584#c7). For the fix we would always accumulate deltas upon triggered events and only perform the action once the accumulated delta is greater than or equal to QWheelEvent::DefaultDeltasPerStep (at the same time resetting the accumulated delta to 0).

Using the scroll wheel, the emitted delta is exactly QWheelEvent::DefaultDeltasPerStep, such that the action is performed at each turn of the wheel.

Revision history for this message
In , Masayuki (masayuki) wrote :

(In reply to Olli Pettay [:smaug] from comment #85)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #82)
> > > That sounds like a minor problem that could be fixed in this bug or a
> > > follow-up.
> >
> > I think that we should decide it in this bug. Smaug, how do you think? I
> > believe that if we'd swap deltaX/deltaY for default action, DOM wheel event
> > and legacy mouse scroll events should represent it. However, if we do so in
> > default settings, there is an issue mentioned in #4 (below).
>
> Doesn't sound like a minor problem.
> Since widgets on web pages tend to emulate native scrollable areas, they
> should get events similar to
> native viewport or other scrollable areas.
>
> #4 is tricky, but I think we should favor the more common case, so swap.
> How does IE handle this? What does Chrome actually report in delta values?

Looks like that IE/Edge doesn't have this feature since Shift+wheel causes navigating history like us. Chrome just swaps default action (i.e., not touching DOM event values).

I wonder, wheel related events are cancelable, that means that it does NOT represent default action's behavior. So, perhaps, we can just take the Chrome's behavior? (Even if web apps check their implementation only with Chrome, it won't cause any problem only on Firefox.) That's not ideal behavior from a point of view of DOM events, but low risk approach in the real world.

FYI: At bug 1358017 Comment 2, similar issue on vertical writing web pages, dbaron thinks that only changing default action is reasonable.

How about you?

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Ok, if some other browser (happens to be Chrome) doesn't change the delta values, we shouldn't either.

Changed in firefox:
status: Confirmed → In Progress
Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :

Created attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

This patch declares a new pref,
"mousewheel.modifier_to_treat_vertical_wheel_as_horizontal_scroll", this takes
a keycode of modifier keys as its value. The default value is 16 (NS_VK_SHIFT)
on Windows, Linux and Android and 0 (meaning disabled) on macOS. The reason
why this new feature is disabled on macOS is, if user turns vertical wheel of
mice with Shift key, macOS notifies us of horizontal scroll events. So, we
don't need to care this feature by ourselves on macOS.

If a modifier keycode is specified to the pref as expected, EventStateManager
and EventStateManager::WheelPrefs treats vertical wheel operation as horizontal
scroll when coming wheel event without the modifier causes scroll.

In such case, EventStateManager::WheelPrefs::NeedToTreatAsHorizontalScroll()
returns true. Otherwise, false. If this returns true, default action handler
of wheel events such as EventStateManager::PostHandleEvent() and
IAPZCTreeManager::ReceiveInputEvent() swaps deltaX values and deltaY values of
coming wheel event temporarily and restore them.
AutoTemporarilyWheelDeltaSwapper in WheelHandlingHelper guarantees this
restoring.

So, this patch does NOT change any wheel event information on web apps. Only
changes its default action. This is same behavior as Chromium.

Note that with this patch, users cannot navigate the tab's history with
Shift + vertical-wheel in the default settings. However, I guess that the
usage of this feature is less than the number of users who wants to scroll
contents horizontally with vertical mouse wheel. Of course,
Shift + horizontal-wheel is available for navigating tab's history even after
this change.

Review commit: https://reviewboard.mozilla.org/r/186794/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186794/

Revision history for this message
In , Masayuki (masayuki) wrote :

I realized that due to e10s mochitests disabled, we won't test the default actions of wheel events without APZ. I manually ran the test without non-e10s mochitests. Then, I confirmed that it's all green.

On the other hand, there is a known bug in the patch. The multiplier pref for X axis is applied to deltaY if the wheel event should be treated as horizontal scroll. So, WheelEvent.deltaY may see odd delta value with it especially when the multiplier X is negative (meaning scroll direction is reversed) but multiplier Y is positive. With more complicated code, I can fix it, but I don't know if it's enough worthwhile...

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

hmm, yeah, perhaps that isn't even a bug but a feature.

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :
Download full text (4.2 KiB)

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review192674

This is complicated enough, that I think I should re-read this after those small nits are fixed.

::: commit-message-19b32:26
(Diff revision 1)
> +restoring.
> +
> +So, this patch does NOT change any wheel event information on web apps. Only
> +changes its default action. This is same behavior as Chromium.
> +
> +Note that with this patch, users cannot navigate the tab's history with

Could you explain this. Currently shift+vertical wheel is back-forward. Does this new behavior override that feature always or only when it is enabled explicitly?

How is shift+horizontal supposed to work for back-forward if shift+vertical is for scrolling? One often gets both horizontal and vertical scrolling when using touchpad.

::: dom/events/EventStateManager.h:652
(Diff revision 1)
> * If an .override_x value is -1, same as the
> * corresponding mActions value.
> */
> Action mOverriddenActionsX[COUNT_OF_MULTIPLIERS];
>
> + // XXX Modifier is better than Modifiers. However, it's defined in

That seems like a silly reason to not use Modifier.
If you're really worried about the build time change, why not just move Modifier to its own header?

::: dom/events/EventStateManager.cpp:5691
(Diff revision 1)
> + }
> +
> + Index index = GetIndexFor(aEvent->mModifiers &
> + ~mModifierToTreatVertialWheelAsHorizontalScroll);
> + Init(index);
> + // We need to cache this result in the widget. Some methods of this class

Cache the result in the widget? I don't see anything stored in the widget.

::: dom/events/EventStateManager.cpp:5710
(Diff revision 1)
> return INDEX_DEFAULT;
> }
> + if (!NeedToTreatAsHorizontalScroll(aEvent)) {
> + return GetIndexFor(aEvent->mModifiers);
> + }
> +#if 0

No ifdef 0 code, please

::: dom/events/EventStateManager.cpp:5837
(Diff revision 1)
> Index index = GetIndexFor(aEvent);
> Init(index);
>
> + // If the event should be treated as horizontal wheel operation, deltaY
> + // should be applied mMultiplierX. Note that deltaX and deltaZ are always
> + // 0 in such case. Therefore, we only need to use temporary variable only

Why they are 0?

::: dom/events/EventStateManager.cpp:5842
(Diff revision 1)
> + // 0 in such case. Therefore, we only need to use temporary variable only
> + // for deltaY. Additionally, if the event is being handled by default
> + // handler, the deltaX and deltaY may be swapped. Therefore, we need to
> + // use mMultiplierX for deltaY only when the event should be treated as
> + // horizontal scroll and mDeltaY isn't 0.
> + auto multiplierForDeltaY = mMultiplierY[index];

Could you not use auto here. With auto I need to go to the mMultiplierY definition to see what the type of multiplierForDeltaY is.

::: dom/events/EventStateManager.cpp:5886
(Diff revision 1)
> aEvent->mOverflowDeltaX /= mMultiplierX[index];
> }
> - if (mMultiplierY[index]) {
> - aEvent->mOverflowDeltaY /= mMultiplierY[index];
> +
> + // If the event shoul...

Read more...

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review192730

::: commit-message-19b32:26
(Diff revision 1)
> +restoring.
> +
> +So, this patch does NOT change any wheel event information on web apps. Only
> +changes its default action. This is same behavior as Chromium.
> +
> +Note that with this patch, users cannot navigate the tab's history with

Yes, when the new pref is enabled, the modifier (default value is Shift on non-macOS platforms) with vertical wheel overrides the default action.

the modifier with horizontal wheel or diagonal wheel operation works exactly same as current build. See NeedToTreatAsHorizontalScroll() implementation for the detail.

::: dom/events/EventStateManager.h:652
(Diff revision 1)
> * If an .override_x value is -1, same as the
> * corresponding mActions value.
> */
> Action mOverriddenActionsX[COUNT_OF_MULTIPLIERS];
>
> + // XXX Modifier is better than Modifiers. However, it's defined in

Sure. I'll move it to EventForwards.h.

::: dom/events/EventStateManager.cpp:5691
(Diff revision 1)
> + }
> +
> + Index index = GetIndexFor(aEvent->mModifiers &
> + ~mModifierToTreatVertialWheelAsHorizontalScroll);
> + Init(index);
> + // We need to cache this result in the widget. Some methods of this class

Ah, I meant in WidgetWheelEvent.

::: dom/events/EventStateManager.cpp:5710
(Diff revision 1)
> return INDEX_DEFAULT;
> }
> + if (!NeedToTreatAsHorizontalScroll(aEvent)) {
> + return GetIndexFor(aEvent->mModifiers);
> + }
> +#if 0

Oops, sorry and nice catch!

::: dom/events/EventStateManager.cpp:5837
(Diff revision 1)
> Index index = GetIndexFor(aEvent);
> Init(index);
>
> + // If the event should be treated as horizontal wheel operation, deltaY
> + // should be applied mMultiplierX. Note that deltaX and deltaZ are always
> + // 0 in such case. Therefore, we only need to use temporary variable only

Because NeedToTreatAsHorizontalScroll() checks |if (aEvent->mDeltaX || !aEvent->mDeltaY || aEvent-DeltaZ)| and return false if it's true.

Revision history for this message
In , Masayuki (masayuki) wrote :

Oops, I used wrong form to reply to the review comments :-(

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review192674

> Could you explain this. Currently shift+vertical wheel is back-forward. Does this new behavior override that feature always or only when it is enabled explicitly?
>
> How is shift+horizontal supposed to work for back-forward if shift+vertical is for scrolling? One often gets both horizontal and vertical scrolling when using touchpad.

Yes, when the new pref is enabled, the modifier (default value is Shift on non-macOS platforms) with vertical wheel overrides the default action.

the modifier with horizontal wheel or diagonal wheel operation works exactly same as current build. See NeedToTreatAsHorizontalScroll() implementation for the detail.

> That seems like a silly reason to not use Modifier.
> If you're really worried about the build time change, why not just move Modifier to its own header?

Sure. I'll move it to EventForwards.h.

> Cache the result in the widget? I don't see anything stored in the widget.

Ah, I meant in WidgetWheelEvent.

> No ifdef 0 code, please

Oops, sorry and nice catch!

> Why they are 0?

Because NeedToTreatAsHorizontalScroll() checks |if (aEvent->mDeltaX || !aEvent->mDeltaY || aEvent-DeltaZ)| and return false if it's true.

Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186794/diff/1-2/

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :
Download full text (3.4 KiB)

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review193228

Still r-, because I was surprised to see
"expected: kScrollDown | kScrollLeft" in case there is both vertical and horizontal scrolling with modifier.
Please explain or fix.

::: dom/events/test/window_wheel_default_action.html:1830
(Diff revision 2)
> +
> + const kTests = [
> + // Without modifier key, the default's action should be used.
> + // I.e., vertical scroll is expected normally.
> + { defaultActions: {
> + default: 1, with_shift: 2, with_control: 3

What these numbers are? 1, 2, 3? Please add some comment

::: dom/events/test/window_wheel_default_action.html:1853
(Diff revision 2)
> + deltaX: 0, deltaY: -1.0,
> + lineOrPageDeltaX: 0, lineOrPageDeltaY: -1,
> + shiftKey: false, ctrlKey: false, altKey: false },
> + expected: kScrollUp
> + },
> + // If only the modifier to treat vertical wheel as horizontal scroll,

I don't understand this sentence.

::: dom/events/test/window_wheel_default_action.html:1914
(Diff revision 2)
> + deltaMultiplierX: 1.0,
> + deltaMultiplierY: 1.0,
> + event: { deltaMode: WheelEvent.DOM_DELTA_LINE,
> + deltaX: 0, deltaY: 1.0,
> + lineOrPageDeltaX: 0, lineOrPageDeltaY: 1,
> + shiftKey: true, ctrlKey: true, altKey: false },

Oh, modifier in the JS object isn't about the event but about the pref. Could you document that somewhere

::: dom/events/test/window_wheel_default_action.html:1981
(Diff revision 2)
> + deltaX: 0, deltaY: -1.0,
> + lineOrPageDeltaX: 0, lineOrPageDeltaY: -1,
> + shiftKey: true, ctrlKey: true, altKey: false },
> + expected: kScrollUp
> + },
> + // If the wheel is operated diagonally, shouldn't treat the deltaY as

This sounds wrong. If the pref is set, and modifier is pressed, why should viewport scroll ever horizontally? I don't see such behavior in Chrome on Windows

::: dom/events/test/window_wheel_default_action.html:2184
(Diff revision 2)
> ["mousewheel.default.action.override_x", -1],
> ["mousewheel.with_shift.action", 2], // history
> ["mousewheel.with_shift.action.override_x", -1],
> ["mousewheel.with_control.action", 3], // zoom
> - ["mousewheel.with_control.action.override_x", -1]]},
> + ["mousewheel.with_control.action.override_x", -1],
> + ["mousewheel.modifier_to_treat_vertical_wheel_as_horizontal_scroll", 0]]},

I don't understand this. Why 0? Doesn't that disable the feature.

::: modules/libpref/init/all.js:2759
(Diff revision 2)
> +// to one of 16 (Shift), 17 (Ctrl), 18 (Alt/Option) and 224 (Command), the
> +// corresponding modifier key with vertical scroll is treated as horizontal
> +// scroll. Note that this setting may override the action set by
> +// "mousewheel.with_*.action" and mousewheel.with_*.action.override_x. E.g.,
> +// when this is set to 16 (Shift) and "mousewheel.with_shift.action" is set to
> +// 2 (History Navigation), try to scrolling vertically with Shift key...

Read more...

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review193228

Because the tests set default action of both without any modifier and with shift to "scroll":
> default: 1, with_shift: 1, with_control: 3
Therefore, even if the new pref overrides "default"'s default action, it should be ignored (i.e., default action of with_shift should be executed) since the wheel operation is not only for vertical-wheel.

In other words, if with_shift is 2 as the default settings of non-macOS platforms, the diagonal wheel operation with Shift should cause navigating history. However, it's impossible to test it with this automated test.

> What these numbers are? 1, 2, 3? Please add some comment

Okay, I'll replace them with constants.

> I don't understand this sentence.

I meant "If only the modifier to trreat vertical wheel as horizontal scroll is active,"

> This sounds wrong. If the pref is set, and modifier is pressed, why should viewport scroll ever horizontally? I don't see such behavior in Chrome on Windows

On macOS, we may receive diagonal wheel operation events. In such case, we should not respect the new pref because the new pref is for legacy mouse device which only supports vertical wheel operation but the user's device is obviously supports horizontal wheel operation in this case. Do you think that we should enable this hack even in this case?

> I don't understand this. Why 0? Doesn't that disable the feature.

Yes. Because if we set a modifier key code here, I need to rewrite existing tests. And that needs special handling with this pref. I.e., that causes making the existing tests really complicated. Therefore, I think that we should keep running existing tests with disabling the new pref. But the new feature should be tested by the new tests.

Revision history for this message
In , Masayuki (masayuki) wrote :

Requesting ni? for the above reply from MozReview.

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review193228

> Oh, modifier in the JS object isn't about the event but about the pref. Could you document that somewhere

I'll rename it instead of documenting it.

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #108)

> On macOS, we may receive diagonal wheel operation events. In such case, we
> should not respect the new pref because the new pref is for legacy mouse
> device
How so? Isn't it for touchpads too.

> which only supports vertical wheel operation but the user's device is
> obviously supports horizontal wheel operation in this case. Do you think
> that we should enable this hack even in this case?
I don't know what hack.

How is this supposed to work on Windows + touchpad? On Chrome if shift is pressed, only
horizontal scrolling will happen.

Revision history for this message
In , Masayuki (masayuki) wrote :

(In reply to Olli Pettay [:smaug] from comment #111)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #108)
>
> > On macOS, we may receive diagonal wheel operation events. In such case, we
> > should not respect the new pref because the new pref is for legacy mouse
> > device
> How so? Isn't it for touchpads too.

Oh, really? I remembered as so. Just my memory is wrong or macOS's behavior has been changed (IIRC, starting Sierra, mouse scroll event behavior has been changed, it might be changed at this time).

> > which only supports vertical wheel operation but the user's device is
> > obviously supports horizontal wheel operation in this case. Do you think
> > that we should enable this hack even in this case?
> I don't know what hack.

I think the new feature is hack for the legacy mice.

> How is this supposed to work on Windows + touchpad? On Chrome if shift is
> pressed, only
> horizontal scrolling will happen.

On Windows, native event doesn't support diagonal scroll with an event. So, this case never occurs on Windows actually. (If user tries to scroll content diagonally, both vertical wheel event and horizontal wheel event are fired separately.) Although, it could occur with some touchscreen. However, I don't see wheel events when I scroll content in Gecko with my notebook's touchscreen. (Pan gesture could fire wheel events, but I'm not familiar with this path: https://searchfox.org/mozilla-central/rev/1a4a26905f923458679a59a4be1e455ebc53c333/widget/windows/nsWinGesture.cpp#393,409-410,412-413 )

I'm not sure about GTK. GTK3's event has both delta values in an event. So, it could support diagonal scroll, but I don't have environment that Linux is installed on real machine (not virtual machine).

Anyway, I don't think that we should use this feature when the user's device is obviously non-legacy pointing device, i.e., it supports diagonal scroll. What do you think?

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #112)
> (In reply to Olli Pettay [:smaug] from comment #111)
> > (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #108)
> >
> > > On macOS, we may receive diagonal wheel operation events. In such case, we
> > > should not respect the new pref because the new pref is for legacy mouse
> > > device
> > How so? Isn't it for touchpads too.
>
> Oh, really? I remembered as so. Just my memory is wrong or macOS's behavior
> has been changed (IIRC, starting Sierra, mouse scroll event behavior has
> been changed, it might be changed at this time).
Sorry, I was unclear. I was talking about touchpads on Windows.
I have no idea about MacOS

> I'm not sure about GTK. GTK3's event has both delta values in an event. So,
> it could support diagonal scroll, but I don't have environment that Linux is
> installed on real machine (not virtual machine).
I see, on linux Chrome has odd behavior where it seems to allow vertical scrolling when diagonal scrolling is done. That behavior is really weird when one uses it. Makes scrolling feel like all broken since viewport is scrolled somewhat randomly. It is hard to control whether to do only up-down or left-right scrolling on a touchpad or a bit both.

> Anyway, I don't think that we should use this feature when the user's device
> is obviously non-legacy pointing device, i.e., it supports diagonal scroll.
> What do you think?
But this is enabled on touchpads too, if the pref is set. Could we somehow enable it on mouse input only, and not touchpad?

Revision history for this message
In , Masayuki (masayuki) wrote :

(In reply to Olli Pettay [:smaug] from comment #113)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #112)
> > Anyway, I don't think that we should use this feature when the user's device
> > is obviously non-legacy pointing device, i.e., it supports diagonal scroll.
> > What do you think?
> But this is enabled on touchpads too, if the pref is set. Could we somehow
> enable it on mouse input only, and not touchpad?

Unfortunately, on any platforms, we cannot check if mouse scroll event came from what kind of device.

There are some options:

1. Take current approach -- treat "pure" vertical wheel operation as horizontal scroll and
  1-1. Handle deltaX as is (current approach).
  1-2. Handle deltaX as vertical scroll (I don't like this and incompatible with Chrome).
  1-3. Ignore deltaX, so, if only deltaX is non-zero, does nothing.
2. Treat any vertical wheel operation as horizontal scroll (even if it's diagonal operation) and/but
  2-1. Handle deltaX as vertical scroll (same as 1-2).
  2-2. Ignore deltaX (same as 1-3).

If user operates wheel diagonally but native events come separately, it might be the best to ignore deltaX. E.g., we can avoid performing 2 default actions (e.g., both horizontal scroll and navigating history), but this means that the default action will always be ignored if it matches with new pref's modifier.

Revision history for this message
In , Masayuki (masayuki) wrote :

2-3. Handle deltaX as horizontal scroll too (but may cause too fast or slow scroll with diagonal wheel operation).

I like 1-1, 1-3 or 2-2.

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

1-3 or 2-2 sound reasonable to me.

Revision history for this message
In , Masayuki (masayuki) wrote :

Okay, now, I think that 2-2 is the best here because:

1. if we ban diagonal wheel events with specific modifier key, horizontal scroll might be too slow if the device can cause diagonal scroll easily (even if user expects only vertical scroll).
2. if we also take deltaX as horizontal scroll, horizontal scroll might be too fast if user does diagonal scroll.

Then, we can treat the new action as simple default action. I'll rewrite the implementation of ESM::WheelPrefs.

Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :
Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186794/diff/2-3/

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186794/diff/3-4/

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review195394

::: browser/app/profile/firefox.js:639
(Diff revision 4)
> -pref("mousewheel.with_shift.action", 2);
> +// only vertical wheel but want to scroll horizontally. For such users, we
> +// should provide horizontal scroll with shift+wheel (same as Chrome).
> +// However, shift+wheel was used for navigating history. For users who want
> +// to keep using this feature, let's enable it with alt+wheel. This is better
> +// for consistency with macOS users.
> +pref("mousewheel.with_shift.action", 4);

ok, this is of course a change to the default behavior, but we do have some time to get feedback

::: dom/events/EventStateManager.cpp:3298
(Diff revision 4)
> - // When APZ is enabled, the actual scroll animation might be handled by
> - // the compositor.
> - WheelPrefs::Action action;
> if (pluginFrame) {
> MOZ_ASSERT(pluginFrame->WantsToHandleWheelEventAsDefaultAction());
> action = WheelPrefs::ACTION_SEND_TO_PLUGIN;

You change the ordering of whether action is first checked for plugin or apz, and delta is adjusted before the plugin chcek. But I guess that makes sense. But please test (manually) some Flash doing scrolling.

::: dom/events/EventStateManager.cpp:5915
(Diff revision 4)
>
> - *aOutMultiplierX = mMultiplierX[index];
> - *aOutMultiplierY = mMultiplierY[index];
> + // If the event should be treated as horizontal wheel operation, deltaY
> + // should be multiplied by mMultiplierY, however, it might be moved to
> + // deltaX for handling default action. In such case, we need to treat
> + // mMultiplierX and mMultiplierY as swapped.
> + double multiplierForDeltaX = mMultiplierX[index];

We do have this similar code in several places.
Can you think of anyway to have a helper method to do this all? If not, fine.

::: dom/events/WheelHandlingHelper.h:233
(Diff revision 4)
> + WidgetWheelEvent& mWheelEvent;
> + double mOldDeltaX;
> + double mOldDeltaZ;
> + double mOldOverflowDeltaX;
> + int32_t mOldLineOrPageDeltaX;
> + bool mTreatedVertualWheelAsHorizontalScroll;

Vertually? Do you mean Virtually

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

https://reviewboard.mozilla.org/r/186794/#review195394

> You change the ordering of whether action is first checked for plugin or apz, and delta is adjusted before the plugin chcek. But I guess that makes sense. But please test (manually) some Flash doing scrolling.

It's necessary to adjust the wheel deltas before updating wheel transaction which is automatically done in ComputeScrollTarget().

And unfortunately, I don't find Flash contents which have horizontal scroll. As far as I read Action Script documents, Action Script doesn't support horizontal wheel events. Therefore, Flash contents cannot handle horizontal scroll anyway. I don't know about the scrollable contents in Flash. That's what I could find.

> We do have this similar code in several places.
> Can you think of anyway to have a helper method to do this all? If not, fine.

Okay, I did it in new patch.

> Vertually? Do you mean Virtually

Oops! Nice catch!

Revision history for this message
In , Masayuki (masayuki) wrote :

Comment on attachment 8915954
Bug 143038 Make users can scroll contents horizontally with vertical wheel operation with a modifier

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186794/diff/4-5/

Revision history for this message
In , Pulsebot (pulsebot) wrote :

Pushed by <email address hidden>:
https://hg.mozilla.org/integration/autoland/rev/63b547bb4078
Make users can scroll contents horizontally with vertical wheel operation with a modifier r=smaug

Revision history for this message
In , Shindli (shindli) wrote :
Changed in firefox:
status: In Progress → Fix Released
Revision history for this message
In , Alice0775-t (alice0775-t) wrote :

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

Revision history for this message
In , David Hedlund (g-public) wrote :

This add-on can be used until Firefox 58 has been released: Shift + Scroll (Horizontal Scrolling) - https://addons.mozilla.org/en-US/firefox/addon/shift-scroll/

Revision history for this message
In , Masayuki (masayuki) wrote :

(In reply to public from comment #128)
> This add-on can be used until Firefox 58 has been released: Shift + Scroll
> (Horizontal Scrolling) -
> https://addons.mozilla.org/en-US/firefox/addon/shift-scroll/

Firefox 56 and earlier, it can be implemented by XUL addons, but Gecko 58+ has this feature. So, only with Firefox 57, you cannot use this feature.

Revision history for this message
In , Zjz-w (zjz-w) wrote :

By navigating the source code, I believe I have figured out how to resolve bug 1438794 and would like to have a try to work on the "first good" bug for myself. Hope someone would assign bug 1438794 for me.

Paul White (paulw2u)
Changed in firefox (Ubuntu):
status: Confirmed → Fix Released
Changed in firefox:
importance: Wishlist → Medium
Revision history for this message
In , Mozilla+bmo (mozilla+bmo) wrote :

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

Displaying first 40 and last 40 comments. View all 137 comments or add a comment.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.