Closed Bug 1708829 Opened 3 years ago Closed 3 years ago

Cannot zoom 3D Models with mouse wheel anymore

Categories

(Core :: DOM: Events, defect, P2)

Firefox 88
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- verified

People

(Reporter: mail, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: dev-doc-complete, regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

Place pointer over 3D model on webpage and scroll mouse wheel to zoom in/out.

Examples (click customize button):
https://demo.spiff.com.au/collections/complex-products/products/watch
https://store.zakeke.com/collections/all/products/pillow

Actual results:

3D model now zooms all the way out after 1 click of mouse wheel (either direction) and will stay zoomed out.

No longer able to zoom in/out starting with Firefox 88 (windows only), but this still works for Firefox 88 (Mac).

Expected results:

3D model should zoom in/out with mouse wheel when pointer place over it.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e897247f1afc2ccc6d25b44ad0b1830cac87386b&tochange=021dcc5231fb07764c68c12310e23e14a35527cd

I confirmed setting dom.event.wheel-deltaMode-lines.disabled to false will fix the issue.(require restart browser)

Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → DOM: Events
Product: Firefox → Core
Regressed by: 1392460

The reason why it doesn't happen in other browsers is:

if (mouseWheelLegacyEvent.wheelDelta) {
    wheelDelta = mouseWheelLegacyEvent.wheelDelta;
} else {
    wheelDelta = -(event.deltaY || event.detail) * 60;
}

wheelDelta is non-standard, but it seems WebKit / Blink implement it. :-(

Flags: needinfo?(emilio)

The behavior for non-trusted events matches Safari (Chrome does return
deltaX in that case, which seems pretty bogus, because the sign of the
wheelDelta* is the opposite as the delta* props).

Assignee: nobody → emilio
Flags: needinfo?(emilio)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Severity: -- → S3
Priority: -- → P2
Flags: needinfo?(emilio)

Masayuki, I think the patch should be in good shape now. I

Flags: needinfo?(emilio)

Since this implements a new attribute:

Keywords: dev-doc-needed
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/044c2060da7d
Expose WheelEvent.wheelDelta{,X,Y} for compat with other engines. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/28faebd5fc72
Make sure that delta* getters account for scroll speed override correctly. r=masayuki

Backed out for causing mochitest plain failures in test_continuous_wheel_events

Backout link: https://hg.mozilla.org/integration/autoland/rev/89776d7bf45e353e4c89f4e2554104cd6037d0b3

Push with failures

Failure log

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ad1a92adf2b
Expose WheelEvent.wheelDelta{,X,Y} for compat with other engines. r=masayuki
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/84821aa766ab
Make sure that delta* getters account for scroll speed override correctly. r=masayuki
Flags: needinfo?(emilio)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

Probably not worth the risk for an uplift.

Flags: needinfo?(emilio)

Testing on macOS with high-precision wheel mouse and low-precision mouse, the former result is exactly same as Chrome and Safari. On the other hand, yeah, as we know, the wheelDeltaY value is different with the latter mouse. Per notch of my mouse, 48 on Gecko, 120 on Chrome and 12 on Safari. But the divided value of wheelDeltaY / deltaY are almost same as Safari, and the value is consistent with the result of high-precision wheel mouse.

On Windows and Linux, as we know, except the non-100% display's issue, our wheelDeltaY is exactly same as Chrome.

Perhaps, some our users may experience slower scroll or something with web apps which handle wheelDelta* rather than delta* because of the bug of Chrome. If that makes the users feel Firefox itself is slower than Chrome, I think that we should fix Chrome's bug...

Anyway, marking this as VERIFIED since the result is what I expected at reviewing them.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

I'm documenting this fix for FF90 - docs tracking in https://github.com/mdn/content/issues/5385

My understanding is that mousewheel zooming stopped working in FF88 (on Windows) and "some problem" has been fixed that means it now works on FF90. However it isn't entirely clear to me from above what stopped working. Can you help me understand what external users need to know about this?

Looking at the above my assumption is that it is something to do with WheelEvent. However the MDN docs indicate that WheelEvent supports deltaX, Y, Z values and according to browser-compat has done so since FF17.
I would guess this was some regression that broke the output, but then this talks about deprecated APIs etc.

FWYI Assuming it is due to WheelEvent behaviour breaking, I would normally update BCD to show that WheelEvent wasn't working between FF88 and FF90. Ideally also with an explanation of what you would get out of the delta values in the problematic releases.

Flags: needinfo?(emilio)

(In reply to Hamish Willee from comment #15)

I'm documenting this fix for FF90 - docs tracking in https://github.com/mdn/content/issues/5385

My understanding is that mousewheel zooming stopped working in FF88 (on Windows) and "some problem" has been fixed that means it now works on FF90. However it isn't entirely clear to me from above what stopped working. Can you help me understand what external users need to know about this?

Yeah, this is not quite right.

Mousewheel zooming in some pages broke, because we aligned Firefox with other browsers and started returning pixels rather than lines for WheelEvent.deltaY, for example, and other browsers weren't using that codepath because other browsers supported a legacy attribute (WheelEvent.wheelDelta). So in this bug I implemented that legacy attribute so that broken websites would follow the same codepath on Firefox as on other browsers.

Does that help? For most websites, which shouldn't be using those legacy attributes, then it shouldn't matter. But basically what this changes is Firefox added support for the legacy WheelEvent.wheelDelta* getters. These don't seem to be documented in MDN to begin with looking at: https://developer.mozilla.org/en-US/docs/Web/API/WheelEvent (and maybe that's ok, as they're non-standard and legacy).

Oh, looking a bit deeper, it is somewhat documented here: https://developer.mozilla.org/en-US/docs/Web/API/Element/mousewheel_event#wheeldelta_wheeldeltax_and_wheeldeltay_value

Flags: needinfo?(emilio)

Hello,

I can reproduce this issue on Fx. 90.0a1 (BuildID: 20210430214504).

I can confirm that this issue is fixed on Windows 10 x64 on Fx 91.0a1 (20210607214637), Fx Beta 90.0b4.

Flags: qe-verify+

Hi Emilio,
Thank you very much - most helpful. So my thinking then is:

  1. Docs: Fix is to update the browser compat data for MouseWheelEvent to show addition of support for this API in FF90. That appears to be where wheelDeltaX etc are defined. Correct?
  2. Release note The real story is a bit complicated to tell in the release note - "we changed something to improve compatibility which broke compatibility for some sites, so we added a deprecated API to support these sites" :-). But how about:

    Firefox 90 adds support for the (deprecated) MouseWheelEvent. This restores Firefox compatibility for a small subset of pages that were broken by recent compatibility improvements to WheelEvent, but which are able to handle these legacy events.

  3. While I'm here, looking at WheelEvent we don't actually define what pixels, lines, and pages mean. Would I be correct in thinking that they are:
    • Pixels - each mouse wheel click is proportional to some number of pixels. Since Pixels are consistent across the screen this will result in some quite predictable movement.
    • Lines - each mouse click corresponds to a movement in "lines". So one little click goes up one line of text. Sounds complicated since what is a line kind of depends on what is on screen right?
    • Pages - each mouse wheel click scrolls a whole page of content.

You can probably also close https://bugzilla.mozilla.org/show_bug.cgi?id=1380217 following this bug.

Flags: needinfo?(emilio)

(In reply to Hamish Willee from comment #18)

Hi Emilio,
Thank you very much - most helpful. So my thinking then is:

  1. Docs: Fix is to update the browser compat data for MouseWheelEvent to show addition of support for this API in FF90. That appears to be where wheelDeltaX etc are defined. Correct?

No, it's in WheelEvent, which is the standard version of the old MouseWheelEvent.

  1. Release note The real story is a bit complicated to tell in the release note - "we changed something to improve compatibility which broke compatibility for some sites, so we added a deprecated API to support these sites" :-). But how about:

    Firefox 90 adds support for the (deprecated) MouseWheelEvent. This restores Firefox compatibility for a small subset of pages that were broken by recent compatibility improvements to WheelEvent, but which are able to handle these legacy events.

We should mention that we added support for WheelEvent.wheelDelta* rather than MouseWheelEvent. These properties live in WheelEvent.

  1. While I'm here, looking at WheelEvent we don't actually define what pixels, lines, and pages mean. Would I be correct in thinking that they are:
    • Pixels - each mouse wheel click is proportional to some number of pixels. Since Pixels are consistent across the screen this will result in some quite predictable movement.
    • Lines - each mouse click corresponds to a movement in "lines". So one little click goes up one line of text. Sounds complicated since what is a line kind of depends on what is on screen right?

Depends on what the app considers a "line" to be, yeah. For example the Firefox tab bar considers a tab to be a line. The default value in Gecko is usually something depending on the font-size / line-height of the scrollable box.

  • Pages - each mouse wheel click scrolls a whole page of content.

You can probably also close https://bugzilla.mozilla.org/show_bug.cgi?id=1380217 following this bug.

Good point, thanks!

Flags: needinfo?(emilio)
Regressions: 1751950
Blocks: 1751967
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: