Comment 113 for bug 1228250

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/#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 causes

s/try to//

::: widget/MouseEvents.h:597
(Diff revision 2)
> mLineOrPageDeltaX : mLineOrPageDeltaY;
> }
>
> + void SwapDeltaXAndDeltaY()
> + {
> + auto tmpDelta = mDeltaX;

I guess you could use std::swap here