Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openstreetmap.org - Mouse wheel does not zoom into the page #73148

Open
kaligrafy opened this issue May 6, 2021 · 38 comments
Open

openstreetmap.org - Mouse wheel does not zoom into the page #73148

kaligrafy opened this issue May 6, 2021 · 38 comments
Assignees
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine os-mac Issues only happening on macOS. priority-normal severity-important A non-core broken piece of functionality, not behaving the way you would expect.
Milestone

Comments

@kaligrafy
Copy link

kaligrafy commented May 6, 2021

URL: https://www.openstreetmap.org/edit#map=15/45.5535/-73.5994

Browser / Version: Firefox 88.0
Operating System: Mac OS X 10.15
Tested Another Browser: Yes Chrome

Problem type: Something else
Description: Since firefox 88, the mousewheel just scroll down instead of zooming in or out in openstreetmap editor Id. I tried going back to version 87, and everything works fine, so something has changed in the mousewheel behaviour in firefox 88.
Steps to Reproduce:
Since firefox 88, the mousewheel just scroll down instead of zooming in or out in openstreetmap editor Id. I tried going back to version 87, and everything works fine, so something has changed in the mousewheel behaviour in firefox 88.

Browser Configuration
  • None

From webcompat.com with ❤️

@webcompat-bot webcompat-bot added this to the needstriage milestone May 6, 2021
@webcompat-bot webcompat-bot added browser-firefox engine-gecko The browser uses the Gecko rendering engine priority-normal labels May 6, 2021
@softvision-raul-bucata softvision-raul-bucata added the os-mac Issues only happening on macOS. label May 11, 2021
@softvision-raul-bucata softvision-raul-bucata changed the title openstreetmap.org - see bug description openstreetmap.org - Mouse wheen does not zoom into the page May 14, 2021
@softvision-raul-bucata softvision-raul-bucata added the severity-important A non-core broken piece of functionality, not behaving the way you would expect. label May 14, 2021
@softvision-raul-bucata softvision-raul-bucata changed the title openstreetmap.org - Mouse wheen does not zoom into the page openstreetmap.org - Mouse wheel does not zoom into the page May 14, 2021
@softvision-raul-bucata
Copy link

We appreciate your report. I was able to reproduce the issue. Using the scroll wheel does not zoom into the page:

Screenshot at May 14 18-18-52

Tested with:
Browser / Version: Firefox Release 88.0 (64-bit)/ Firefox Nightly 90.0a1 (2021-05-13) (64-bit)/ Chrome Version 90.0.4430.85
Operating System: Mac OSX 10.15.6

Notes:

  1. Reproducible regardless of the status of ETP
  2. Reproducible on the latest build of Firefox Nightly
  3. Works as expected using Chrome

Moving this to NeedsDiagnosis for further investigations.

@ksy36
Copy link
Contributor

ksy36 commented May 20, 2021

I get the same behaviour in Chrome. Zoom in is possible only by clicking "Zoom in to edit" and by clicking +/- . Mouse wheel only scrolls down or up indeed. And if I use trackpad, it zooms in/out in both browsers.

This is probably related to openstreetmap/iD#5550

@karlcow wonder if you get different behaviour between Chrome and Firefox?

@kaligrafy
Copy link
Author

kaligrafy commented May 20, 2021

mouse wheel was zooming in and out in firefox 87, but not in firefox 88. In Chrome (Version 90.0.4430.212 (Official Build) (x86_64)), I get the correct behaviour (zoom in and out)

@karlcow
Copy link
Member

karlcow commented May 24, 2021

Unfortunately I can't test this one, because I do not own a mouse 🐭
BUT with the trackpad on the macbook both for Gecko (Firefox) v.90 and Blink (Chrome/Edge) v.92

  • Two fingers gestures scrolls the view
  • Control + Two fingers activates the zoom
  • Pinch to zoom in/out is working too.

I seem to have a different behavior than @ksy36 here. Did you test on 88? or on 90? 😁

But if there is a difference in between Firefox 88 and Firefox 87, a mozregression would make it possible to reveal the issue.

mozregression --bad 89 --good 86 should make it possible to find out which build, if any, has changed.

@karlcow
Copy link
Member

karlcow commented May 24, 2021

@kaligrafy If you set in about:config

if you change dom.event.wheel-deltaMode-lines.disabled does it work?

@karlcow
Copy link
Member

karlcow commented May 24, 2021

This could be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1392460

@kaligrafy
Copy link
Author

@karlcow setting dom.event.wheel-deltaMode-lines.disabled to false fixes the problem in firefox 88.0.1.

@karlcow
Copy link
Member

karlcow commented May 24, 2021

@emilio so that's a regression of https://bugzilla.mozilla.org/show_bug.cgi?id=1392460
Do you want me to open a separate bug on bugzilla?

@wisniewskit On the side of the webcompat team, I wonder if we can create a site interventions for this.

@wisniewskit
Copy link
Member

I think we should be able to, if there isn't an even better mechanism for it already. I'll investigate tomorrow. Let's file a bug if emilio agrees that an intervention is the way to go.

@karlcow
Copy link
Member

karlcow commented May 25, 2021

ok the magic happens in https://www.openstreetmap.org/assets/id-cb3f7d4c3875433a329179a3b331e9fb5a67cbc5a7faf800ba072198b8c8472b.js

Capture d’écran 2021-05-25 à 09 28 54

also

  function defaultWheelDelta$1(e) {
    return - e.deltaY * (1 === e.deltaMode ? 0.05 : e.deltaMode ? 1 : 0.002)
  }

@liamengland1
Copy link

https://github.com/openstreetmap/iD/blob/1ee45ee1f03f0fe4d452012c65ac6ff7649e229f/modules/renderer/map.js#L471
https://github.com/openstreetmap/iD/blob/1ee45ee1f03f0fe4d452012c65ac6ff7649e229f/modules/util/zoom_pan.js

@karlcow
Copy link
Member

karlcow commented May 25, 2021

@emilio
Copy link

emilio commented May 25, 2021

So, if OSM reads deltaY before deltaMode like they're doing, they should be getting a deltaMode of pixels, right? Why is that not working as expected?

@emilio
Copy link

emilio commented May 25, 2021

Also, it's fixed on Nightly, right?

@karlcow
Copy link
Member

karlcow commented May 25, 2021

@softvision-raul-bucata can you reproduce in nightly

@softvision-raul-bucata
Copy link

@karlcow I can still reproduce the issue on the latest build of Firefox Nightly, with all the settings in about:config set to default. Still works as expected using Chrome for me.

@emilio
Copy link

emilio commented May 26, 2021

Hmm, it works for me on Nightly (Linux, FWIW)

@karlcow
Copy link
Member

karlcow commented May 27, 2021

Maybe the codepath is different depending on the OS. I have seen things specific for macOS, but I didn't see anything obvious for windows.

PointerEvent in window returns false.
https://github.com/openstreetmap/iD/blob/1ee45ee1f03f0fe4d452012c65ac6ff7649e229f/modules/renderer/map.js#L72-L73

so it is using d3_zoom

@kaligrafy
Copy link
Author

Is there a temporary fix I can make to my id editor fork in the meantime?

@karlcow
Copy link
Member

karlcow commented Jul 22, 2021

@kaligrafy did you have a chance to try on Firefox Nightly? (92.0a1 (2021-07-21) (64-bit) is the latest version?)
https://www.mozilla.org/en-US/firefox/all/#product-desktop-nightly

@ksy36 did you reproduce on macOS or linux?

@kaligrafy
Copy link
Author

The issue is still there on nightly. Sorry :-(

@kaligrafy
Copy link
Author

Maybe it can help, but pressing the option key while using the mouse wheel in the id editor fixes the problem (it zooms instead of panning)

@mbrzakovic
Copy link

The issue was that on mac os firefox, deltaX and deltaY were Integer values for both two finger moving and mouse scroll events.
Therefore both actions were treated as 'two finger moving' in iD code.

I've implemented a fix [hosted currently on iD dev endpoint] in iD to disable 'two finger panning' for mac os - firefox, but in general i would like to know the following:

How to differentiate between mouse scroll and two finger movement on mac os firefox?

@karlcow
Copy link
Member

karlcow commented Aug 16, 2021

@mbrzakovic That's a good question

How to differentiate between mouse scroll and two finger movement on mac os firefox?

I think we should probably provide an example for making it clear to people how this is all working.

@emilio @masayuki-nakano

The function zoomPan has too much browser sniffing to my taste to be resilient to any changes. 😢
https://github.com/openstreetmap/iD/blob/2270b5dd9c5c461337a7f1a0655e2d9eea3650ec/modules/renderer/map.js#L461-L638

all of this doesn't give a very good story for interop.

@masayuki-nakano
Copy link

I'm confused at this reported issue. If returning pixels from delta values and delta mode on Firefox, it must have already been broken on Firefox on macOS (i.e., 87 or earlier) if user uses a mouse having high precision mouse wheel or the trackpad of Apple.

And the function does not handle normal (expected) case first, so, I still don't understand what should be, and what's wrong...

@masayuki-nakano
Copy link

Ah, 88 is not the first release of the new wheel event...

If this is reproducible only on macOS, does this affect the result?
https://searchfox.org/mozilla-central/rev/d3683dbb252506400c71256ef3994cdbdfb71ada/modules/libpref/init/StaticPrefList.yaml#3693-3704

@emilio
Copy link

emilio commented Aug 16, 2021

Generally making decisions about the source of an event based on whether the delta is an integer value or not seems pretty finicky... What guarantees that?

@karlcow
Copy link
Member

karlcow commented Aug 17, 2021

It seems the code for the zoomPan has layers over layers of legacy code.

I'm trying to understand this part

            // Normalize mousewheel scroll speed (Firefox) - #3029
            // If wheel delta is provided in LINE units, recalculate it in PIXEL units
            // We are essentially redoing the calculations that occur here:
            //   https://github.com/d3/d3-zoom/blob/78563a8348aa4133b07cac92e2595c2227ca7cd7/src/zoom.js#L203
            // See this for more info:
            //   https://github.com/basilfx/normalize-wheel/blob/master/src/normalizeWheel.js

The line 203 is

k = Math.max(k0, Math.min(k1, t.k * Math.pow(2, -event.deltaY * (event.deltaMode ? 120 : 1) / 500))),

and the more info points to a code from Facebook from 2015, Facebook, Inc. not adjuster since June 2016.
And that's probably part of the issue(s).

The code starts with

            if (source.deltaMode === 1 /* LINE */) {
                // Convert from lines to pixels, more if the user is scrolling fast.
                // (I made up the exp function to roughly match Firefox to what Chrome does)
                // These numbers should be floats, because integers are treated as pan gesture below.

folding the code to have a better understanding.

Capture d’écran 2021-08-17 à 21 20 11

@karlcow
Copy link
Member

karlcow commented Aug 18, 2021

@mbrzakovic why does isInteger matter in your code?

Did you see the discussion on
w3c/uievents#181

@mbrzakovic
Copy link

@karlcow I recently took over as interim maintainer for iD.

I am guessing that up until now 'isInteger' was used to differentiate between Mac OS 2-finger panning and mouse scroll on Firefox. (2 finger panning should move the map while mouse scroll should zoom in/out). Please look at https://github.com/openstreetmap/iD/blob/81b7e282c31391fc6457964f437fcab85ef90b6e/modules/renderer/map.js#L562.
(the browser != 'Firefox' part is my recently added short-term fix for this issue)

During debugging I found that for Mac OS firefox dX, dY are integerers for both cases, which does not seem to happen for other browsers.

@karlcow
Copy link
Member

karlcow commented Aug 23, 2021

Someone has laid out the issues
openstreetmap/iD#8649

@LaoshuBaby
Copy link

LaoshuBaby commented Aug 9, 2022

We appreciate your report. I was able to reproduce the issue. Using the scroll wheel does not zoom into the page:

Screenshot at May 14 18-18-52

Tested with: Browser / Version: Firefox Release 88.0 (64-bit)/ Firefox Nightly 90.0a1 (2021-05-13) (64-bit)/ Chrome Version 90.0.4430.85 Operating System: Mac OSX 10.15.6

Notes:

1. Reproducible regardless of the status of ETP

2. Reproducible on the latest build of Firefox Nightly

3. Works as expected using Chrome

Moving this to NeedsDiagnosis for further investigations.

This problem still happen in Firefox latest 103.0.1 (not nightly)

I think openstreetmap/iD#5550 is related.

@denschub denschub modified the milestones: needsdiagnosis, sitewait Aug 9, 2022
@LaoshuBaby
Copy link

LaoshuBaby commented Aug 9, 2022

@denschub Thank you for the action!

I upload 7 record video about my operation in different situation in iD's repository: openstreetmap/iD#5550 (comment) (more information can be seen here)

And I'm using windows 11 so maybe a os-win tag also needed.

Oh, sorry! This is because I wrong use MacOS's UA:

openstreetmap/iD#5550 (comment)

now everything work correctly.

@Zabooby
Copy link

Zabooby commented Feb 7, 2024

This has been fixed, close?

@wisniewskit
Copy link
Member

Yeah, I think it's safe to close this. Thanks for the reminder!

@wisniewskit wisniewskit closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2024
@wisniewskit wisniewskit modified the milestones: sitewait, invalid Feb 7, 2024
@1ec5
Copy link

1ec5 commented Feb 7, 2024

@wisniewskit, if I’m not mistaken, this issue is specific to macOS and still reproduces. @LaoshuBaby was using Windows, which has always been unaffected by the issue. I provided more details in openstreetmap/iD#8649. Since then, a fork of iD worked around this issue, sort of, by adding a user preference that Mac Firefox users must manually select if they want the intended behavior.

@wisniewskit wisniewskit modified the milestones: invalid, sitewait Apr 11, 2024
@wisniewskit
Copy link
Member

Yeah, sorry for the delay here, I'll reopen this for now.

@wisniewskit wisniewskit reopened this Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine os-mac Issues only happening on macOS. priority-normal severity-important A non-core broken piece of functionality, not behaving the way you would expect.
Projects
None yet
Development

No branches or pull requests