Comment 108 for bug 1228250

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.