Closed Bug 1052253 Opened 11 years ago Closed 8 years ago

OS X: Pinch to zoom gesture should map to mousewheel with the control key, like Chrome

Categories

(Core :: Widget: Cocoa, defect, P1)

Product:
Core
Component:
Widget: Cocoa
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: evanw, Assigned: poiru)

References

Details

(Keywords: dev-doc-complete, Whiteboard: tpi:+)

Attachments

(2 files, 3 obsolete files)

Part 2: Map macOS pinch to zoom gesture to mousewheel with the control key
3.44 KB, patch
mstange
: review+
Details | Diff | Splinter Review
Part 1: Fix gestures when using macOS 10.10 or later SDK
5.67 KB, patch
mstange
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36 Steps to reproduce: 1) Obtain a MacBook with a trackpad and OS X 2) Visit http://jsbin.com/fepuficudolo 3) Try to pinch to zoom My settings in about:config for browser.gesture.pinch.* are all the defaults (i.e. empty). Actual results: Firefox does not map the pinch gesture to any JavaScript event. This means that applications which need the pinch gesture do not have the opportunity to override it. Expected results: Chrome maps the pinch gesture to a mousewheel event with the ctrlKey property set to true. Calling preventDefault() on it prevents the browser's page zoom behavior. The relevant bug is http://crbug.com/289887. Apparently this behavior was already available in IE and in Firefox through an extension at the time this decision was made. For more discussion, see https://groups.google.com/a/chromium.org/d/msg/chromium-dev/L_kaBhYFi5U/RIMFBx12dJoJ.
Component: Untriaged → Widget: Cocoa
Product: Firefox → Core
Why not rely on an extension for this behavior now?
Because, as an app developer, I cannot use on a feature that's implemented by an extension if the user doesn't have the extension. Pinch to zoom is a core trackpad feature available on all OS X laptops and even desktops if the user has Apple's external trackpad. It would be a shame if the web platform didn't support such a basic feature. I experimented with a patch over the weekend and it looks like it'd be a few lines of code to add support to Firefox. Is there a good reason for keeping it as an extension?
Attached patch pinch-gesture-support.patch (obsolete) — — Splinter Review
I talked with Steven Michaud and he said to go for it. This patch matches Chrome's behavior almost exactly, except it uses a slightly different formula for the deltaY value that results in the same movement speed as Chrome but doesn't drift over time. Tested and verified: * Pinch speed on a retina display matches pinch speed on a non-retina display * Pinch speed is similar to Chrome * The page gets fake wheel events with the control key down * Calling preventDefault() on fake wheel event stops the default pinch action * Not calling preventDefault() allows the default pinch action * Not calling preventDefault() does not cause the page to scroll All default pinch actions were tested with browser.gesture.pinch.in = cmd_fullZoomReduce and browser.gesture.pinch.out = cmd_fullZoomEnlarge, and then again with both browser.gesture.pinch.in and browser.gesture.pinch.out empty.
Attachment #8474961 - Flags: review?(mstange)
Comment on attachment 8474961 [details] [diff] [review] pinch-gesture-support.patch Hmm, the other places in nsChildView.mm that need to check whether the event was defaultPrevented use the return value of mGeckoChild->DispatchWindowEvent instead of looking at event.mFlags.mDefaultPrevented. Are the two equivalent? Should we be changing the other callers? Why doesn't our normal ctrl+wheel page zooming pick up these new events?
Smaug, Masayuki, do you approve of this change? From the chromium bug it sounds like Firefox on Windows 8 is already doing something like this.
Flags: needinfo?(bugs)
Super ugly, but if others are doing it too, fine.
Flags: needinfo?(bugs)
Comment on attachment 8474961 [details] [diff] [review] pinch-gesture-support.patch I really appreciate the detailed comment in this patch.
Attachment #8474961 - Flags: review?(mstange) → review+
Hmm, dispatching a wheel event with Control key state sounds bad to me... Does Chrome dispatch it? On all platforms? I guess Chrome doesn't so on other platforms. And whether pressing Control key at turning wheel causes scroll depends on the user setting. Therefore, this patch sends wrong message to web developers that they can assume a wheel event with Control key always causes zoom in/out. I believe that we shouldn't do that.
Ah, with default system settings, we never receive native wheel events while Control key is pressed... http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#600
I wonder if "mousewheel.with_control.delta_multiplier_y" is -100, what happens with the patch?
Markus, I thought that too and tried that first, but mDefaultPrevented is not the same thing as the return value of DispatchWindowEvent(). DispatchWindowEvent() always returns true for wheel events regardless of whether the page calls preventDefault() or not. It looks like EventStateManager::PostHandleEvent() always sets the event status to nsEventStatus_eConsumeNoDefault for NS_WHEEL_WHEEL messages. Masayuki, setting "mousewheel.with_control.delta_multiplier_y" to -100 does indeed flip the pinch direction. I agree that control-scrolling for the pinch gesture isn't at all elegant and deltaZ makes more sense. Should I update the patch to use deltaZ instead?
(In reply to Evan Wallace from comment #11) > Masayuki, setting "mousewheel.with_control.delta_multiplier_y" to -100 does > indeed flip the pinch direction. I agree that control-scrolling for the > pinch gesture isn't at all elegant and deltaZ makes more sense. Should I > update the patch to use deltaZ instead? Currently, deltaZ isn't used by any devices even though it's defined in D3E and NSWheelEvent of Cocoa has it. I think that we should not use it for the compatibility with the future. Ideally, we should suggest Zoom event to W3C (HTML5? or D3E?), and all browsers should implement it. Using wheel event is too hacky for this use case. If we do so, web apps can prevent to zoom in/out from any way (e.g., from menu). But it needs a long time. So, I think that if we should provide such way to web developers, we should dispatch "MozZoom" event which is implemented by UIEvent and its detail value indicates the new zoom level. How do you think, smaug? Anyway, I'll file a spec bug.
Flags: needinfo?(bugs)
So in which way does IE report pinch-to-zoom? I thought it was also dispatching wheel+ctrl And no to prefixed events, so, no MozZoom please. Zoom would be perhaps fine, and isn't w3c IndieUI WG planning to have something like that.
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #14) > So in which way does IE report pinch-to-zoom? I thought it was also > dispatching wheel+ctrl Looks like IE desktop *might* dispatch wheel event for this purpose since I cannot catch WM_MOUSEWHEEL message in all windows both Spy++ 32 bit and 64 bit. But I cannot catch WM_GESTURE too... So, I don't know what events IE handles... On Chrome for Windows, my touchpad driver sends WM_MOUSEWHEEL with ctrl key state. So, I'm not sure what Chrome does when it receives pinch gesture event. Anyway, our prefs can customize which modifier can be used for zoom. And also if we dispatch wheel event for the hack, we make wheel event hacky, not a strict event representing device operation. So, I strongly object about using wheel event for this way. > And no to prefixed events, so, no MozZoom please. Zoom would be perhaps > fine, > and isn't w3c IndieUI WG planning to have something like that. Indeed. I've never known IndieUI Events. Fortunately, IndieUI Events do not use detail value for "zoom" event. So, I think that we can use "zoom" event temporarily with UIEvent interface (or MouseEvent interface. (Or it could be useful if we use WheelEvent interface with "zoom" type value)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Is there any way I can be helpful in getting a trackpad pinch event in Firefox? What process should I follow? Who should I talk to? What needs to be coded? This is consistently one of our user's favorite features of our app and is a huge workflow booster (we're writing a drawing app). It's sad that I have to recommend Chrome to our users because Firefox doesn't give users full access to their trackpad.
(In reply to Evan Wallace from comment #16) > Is there any way I can be helpful in getting a trackpad pinch event in > Firefox? What process should I follow? Who should I talk to? What needs to > be coded? This is consistently one of our user's favorite features of our > app and is a huge workflow booster (we're writing a drawing app). It's sad > that I have to recommend Chrome to our users because Firefox doesn't give > users full access to their trackpad. Masayuki-san, can you answer some of these questions?
Flags: needinfo?(masayuki)
I'm not familiar with gesture input handling...
Flags: needinfo?(masayuki)
I'd say we need a new event for this. Chrome's behavior is bizarre, and if possible we shouldn't copy it. https://dvcs.w3.org/hg/IndieUI/raw-file/default/src/indie-ui-events.html#zoomrequest is what the IndieUI has atm, but that looks rather preliminary. Evan, want to ask in IndieUI mailing list about the stability of their spec and how zoomrequest event actually should work with trackpads?
I asked the IndieUI mailing list a week ago but I haven't gotten a reply yet. The list appears dead; there have only been four emails so far this year and none of them have any replies. Who inside Mozilla would be the right person to talk to about this?
I and Masayuki. Note, in general gesture handling events are an IP nightmare, and they were explicitly not included when Web Events working group[1] was created. IndieUI approach is perhaps better since it is higher level than gesture events. I'll ping some IndieUI WG members. [1] http://www.w3.org/2010/webevents/charter/
I lost the irc discussion log, but the idea seems to be that some of the work will be moved to CSS WG and some to WebApps WG. I'll ping jcraig again next week (during W3C TPAC, although I'm participating only remotely)
(In reply to Olli Pettay [:smaug] from comment #22) > I lost the irc discussion log, but the idea seems to be that some of the > work will be moved > to CSS WG and some to WebApps WG. I'll ping jcraig again next week (during > W3C TPAC, although I'm participating only remotely) Did this happen? What was the outcome?
Flags: needinfo?(bugs)
I'm not aware of any new spec work happened here yet.
Flags: needinfo?(bugs)
Evan has an implementation and it works just like it does in Chrome. Try Google Maps on Firefox and then on Chrome, and on you'll see the "Pinch and Zoom" doesn't work in Firefox/Safari(OSX) and does in Chrome. It's really not useful to have Firefox/Safari(OSX) tying pinch and zoom to and un-interceptible browser zoom. Browser apps need access access to the pinch-zoom gesture, and the ctrl+mousewheel works well. I think the Chrome solution is a solid approach for the short term, and spec-ing something more complex will take more time than people are willing to commit here. Why not start with a reasonable solution, and improve upon that later.
Note that this approach works (with Chrome) on Google Maps, Bing Maps and Nokia HERE Maps. It sounds like authors are beginning to treat it as a de facto standard.
Yes, Chrome did horrible job here and added this behavior. Pinch to zoom has nothing to do with wheel :/ So I guess we need to follow what they do.
Rick might have some background information why this was done in blink. (and sorry that I was a bit harsh towards Chrome.)
Sorry, just realized I hadn't replied to this yet. Yes, I implemented a hack - I admit it. But IMHO it was a reasonable pragmatic trade off to solve an important issue where the only realistic alternative for us was to do nothing and leave it broken. I defended it by saying we were just improving cross-platform consistency - Windows trackpad drivers nearly universally send ctrl+wheel events for pinch-zoom, so we were just extending that quirk to Mac. It's totally debatable though, but for us it was the right pragmatic choice and I'm glad we did it (even though it feels a little dirty). Discussion: https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/mousewheel$20byers/chromium-dev/L_kaBhYFi5U/RIMFBx12dJoJ Bug: http://crbug.com/289887 A generic 'zoom' event would definitely be better, but I was highly skeptical we'd ever get something that IE and Apple would implement. Now there's some evidence that could change: https://twitter.com/RickByers/status/618380969343451136. If that happens, I'm confident we could eventually remove the ctrl+wheel hack from Chrome.
(In reply to Olli Pettay [:smaug] from comment #27) > So I guess we need to follow what they do. Can we land the patch?
Flags: needinfo?(bugs)
That is for osx only. We need same behavior on all platforms, at least on those desktop platforms which support zoom. eMagnifyGestureUpdate seem to be used also on Windows and Android. Does Chrome map zoom to ctrl+wheel also on Android?
Flags: needinfo?(bugs)
At least on Windows Chrome has similar behavior. Based on irc comments, Chrome does _not_ have that behavior on Android. Nicely inconsistent. Oh well. So, at least we should change also Windows backend.
Do you mean does touchscreen pinch-zoom fire ctrl-wheel events? No, that would be bad (it's not "wheel" and we wouldn't want to block every frame of pinch-zoom on a cancelable wheel event). Yes it sucks that our only "zoom" API at the moment is input-device specific (touchscreen you have to build it yourself on touch events, touchpad you use wheel). Apparently Safari is extending their proprietary gesture events for this: https://developer.apple.com/library/prerelease/mac/releasenotes/General/WhatsNewInSafari/Articles/Safari_9_1.html#//apple_ref/doc/uid/TP40014305-CH10-SW7. We could jump on board and standardize their API together if you like :-) Or are you really talking about mousewheel support in Chrome for Android? Mouse input is pretty poorly supported in Android at the moment, so if there is some inconsistency between desktop and Android wrt. mouse devices, that's just a bug that I can prioritize fixing.
(In reply to Rick Byers from comment #33) > Do you mean does touchscreen pinch-zoom fire ctrl-wheel events? No, that > would be bad (it's not "wheel" and we wouldn't want to block every frame of > pinch-zoom on a cancelable wheel event). pinch to zoom on touchpad is no more wheel than pinch-zoom on touchscreen. This is all rather magical, so need to reverse engineer what other implementations do. > Apparently Safari is extending their proprietary gesture events for this: > https://developer.apple.com/library/prerelease/mac/releasenotes/General/ > WhatsNewInSafari/Articles/Safari_9_1.html#//apple_ref/doc/uid/TP40014305- > CH10-SW7. We could jump on board and standardize their API together if you > like :-) There were reasons why gesture events were explicitly not part of touchevents or pointerevents working group charter :(
> Apparently Safari is extending their proprietary gesture events for this: That would be very good. Just stuck in situation where I need to properly handle zoom events on touchpads and was very surprised that I can handle it only in Chrome at the moment (at least on OS X). Also, as far as I know, IE/Edge also has some proprietary MSGesture events, which I think are usable with touchscreen. Does anyone has info if they are usable with touchpad?
Flags: needinfo?(redux)
> pinch to zoom on touchpad is no more wheel than pinch-zoom on touchscreen. Btw, I understand why touchpad zoom is associated to Ctrl + Wheel. Since it was designed as a replacement/alternative to mouse, every gesture of it was translated to similar mouse event. E.g. 2 finger move is translated to MouseWheel scrolling. Should it not fire Wheel events and just PointerEvents with 2 pointers? I bet no. While I understand that it's ugly to have Ctrl + Wheel for zoom in general (try Ctrl + Zoom in OS X Firefox on Google Maps, it's treated and zoom on touchpad), it's fine as a temporal solution.
Also here is some research I made about scroll/zoom events on OS X: https://gist.github.com/NekR/9a80ebe73573e11f0351
(In reply to Rick Byers from comment #33) > Apparently Safari is extending their proprietary gesture events for this: > https://developer.apple.com/library/prerelease/mac/releasenotes/General/ > WhatsNewInSafari/Articles/Safari_9_1.html#//apple_ref/doc/uid/TP40014305- > CH10-SW7. We could jump on board and standardize their API together if you > like :-) This has shipped with Safari 9.1 now. Documentation is at https://developer.apple.com/library/mac/documentation/UserExperience/Reference/GestureEventClassReference/
We're using the gesture override for pinch-zoom on Safari now, and it works well. The only browser without pinch-zoom support is now Firefox, and this is one of the last major issues for our web application. We'd be fine using either approach.
Given that others map zoom to wheel + ctrl on desktop, I'm fine with that, but we need to do that consistently on all the desktop platforms. See comment 31 and comment 32. Exposing support for gesture events could have similar problems as standardizing them, maybe. (In reply to Markus Stange [:mstange] from comment #38) > This has shipped with Safari 9.1 now. Documentation is at > https://developer.apple.com/library/mac/documentation/UserExperience/ > Reference/GestureEventClassReference/ Is that mobile Safari only or desktop too? The example has touchevents which hints about mobile only.
(In reply to Olli Pettay [:smaug] from comment #40) > (In reply to Markus Stange [:mstange] from comment #38) > > This has shipped with Safari 9.1 now. Documentation is at > > https://developer.apple.com/library/mac/documentation/UserExperience/ > > Reference/GestureEventClassReference/ > Is that mobile Safari only or desktop too? The example has touchevents which > hints about mobile only. According to http://www.mobilexweb.com/blog/safari-on-ios-9-3-picture-shrink-fit-iphone-se "Since Safari on iOS 2.0 we have non-standard gesture events to detect pinch and rotate gestures on iOS devices usually through gesturechange. These events are also available on Safari on the Mac when the user has a trackpad with gesture support, such as a MacBook or a Magic Mouse/Trackpad." Not sure if/where the documentation has been updated yet at Apple's end to that effect. Also not sure if this is worth standardising, or treating as a platform-specific quirk/feature.
Flags: needinfo?(redux)
Sorry for the noise, but better link: https://webkit.org/blog/6008/new-web-features-in-safari/ "Gesture Events for OS X Already available in WebKit for iOS, gesture events are supported on OS X with Safari 9.1. Gesture events are used to detect pinching and rotation on trackpads."
Priority: -- → P1
Whiteboard: tpi:+
Version: 34 Branch → Trunk
hey kats, is this something we could hook apz up to?
Flags: needinfo?(bugmail)
Whiteboard: tpi:+ → tpi:?
I think long-term we definitely want to hook up pinch-zoom gestures on the OS X trackpad to a "continuous zoom" just like we do on Android. Bug 1245183 is tracking that work, but it won't be done anytime soon. In the meantime though we should probably map it to some sort of gesture events or ctrl+mousewheel (whatever is most compatible with other browsers, I didn't read the whole bug in detail), and this bug is an appropriate place to do that.
Flags: needinfo?(bugmail)
I totally agree. It's been over two years now since we filed this bug, and this is one of the main paint points with our current web design tool on Firefox. It is a definite blocker for our users. I can't imagine using Google Maps and web design tools without working pinch-zoom especially on OSX, and Windows has much improved trackpads of late too. Safari has fixed this with gesture events, but you have to install 10.11.5 just to get at the new Safari release. It's much simpler for our users to just install a new browser, than a whole new OS.
Whiteboard: tpi:? → tpi:+
I think this is a very basic thing to have in a web browser. I've recently been **trying** to switch from chrome to firefox because I believe in it but its just a bit strange that something so basic like this has been discussed for 2 years. I was shocked I couldnt pinch-zoom in google maps...
To be fair, it took us >2 years of complaints from maps users and going around in circles to fix this in Chrome. It's a stupidly annoying space for legal reasons, we finally convinced ourselves to do this pragmatic hack. Something better is definitely still needed.
Can we get any traction on this bug again? Figma works so well on Firefox except for pinch-and-zoom on OSX. For some reason, on Windows pinch-and-zoom does work (gesture support?). Another issue is that "wheel" events don't provide access to the unscaled wheelDeltaX/wheelDeltaY values. deltaX/deltaY are scaled always on Windows by the OS sensitivity factors. We need the actual ticks of the mousehweel, not the scaled versions.
Even Safari which didn't support the ctrl+mousewheel approach added gesture support last year, and we were able to tie into that. Having neither of these in Firefox makes touchpad-based apps feel broken.
stone, do you think you had time to look at this? See comment 40.
Flags: needinfo?(sshih)
I might not be able to find the cause/solution soon but I'd love to take it.
Assignee: nobody → sshih
Flags: needinfo?(sshih)
So it appears that Chrome must just stuff the raw gesture value for pinch-zoom to ctrl+mousehweel conversion. This is a problem, since that value is 4x off (on OSX and probably Windows) from what an actual ctrl+mousewheel user action generates. So now we have to detect this case, but the only way to detect it is to test for fractional values from the conversion vs. integer from an actual user action. Sharing this, since maybe Rick can comment on the implementation (or possibly fix it). It's feels like this was only used to get Google map zoom working, and not for more general cases. Mouse handling and normalization across browsers appears to be a big problem that web developers have to solve, since the browsers haven't seemed to agree to any standard here. That makes for a lot of tricky code. Also inertia events have no flag to detect and ignore to lock out pan from zoom when modifiers are hit. I'm assuming pointer event might have this, and Edge has something in gesture events about inertia.
Can we please land Evan's patch? Yes, it's macOS only, but thithe lack of pinch zoom is a *huge* drawback particularly on Apple devices because they have excellent trackpads and their users are used to the pinch zoom gesture. Google Maps much nicer to use in Chrome because they added this hack.
It seems reasonable to me to match Chrome's behavior, especially if top sites are relying on it, as Comment #26 suggests. (and if we do, it's probably something we should spec somewhere like the compat standard, or via WICG if Edge is interested in implementing).
There is some complexity in interpreting from a gesture -> wheel delta and then back to a gesture. Even on Chrome, it's tricky to tell when the user hit ctrl+wheel, and when Chrome sent sent ctrl down for pinch-zoom. The only identifier is that the key repeat setting is set on the faked ctrl modifier. There are wildly different magnitudes on the Chrome Mac for the two wheel values sent. Pinch-zoom is deltaY = -100*log(gesture) = tiny. We then have to extract that back as a gesture value (-deltaY/100) to send to a exponential zoom. Actual ctrl+wheel sends deltaY and wheelDeltaY often at max (120*devicePixelRatio on Chrome). So if you get your conversions wrong, or mis-identify zoom (actual ctrl+wheel) vs. pinch-zoom incorrectly, then the scale is completely off. It also doesn't help that Firefox doesn't implement wheelDeltaX/Y which are needed for the unscaled notch delta. On Windows, set numLinesToScroll to 1 and then 100, and try device pixel ratio of 100%, 200%, 300% on a high def monitor. Chrome scales by device pixel ratio incorrectly, and Firefox scales delta by numLinesToScroll. Fortunately, Mac hid numLinesToScroll from users. How users normalize mousewheel for pan/zoom/pinch is really challenging. Firefox exhibits numerous other problems on our Mac build. I rotate an optical mouse wheel 10 times before one wheel event is sent to the app, so some accumulator is not triggering off each notch. This doesn't happen on Windows, and each notch sends a wheel event. Then Firefox returns small floats for real wheel with one mouse, and large integers for pinch-zoom on a Samsung laptop, and returns the opposite on a Razer laptop. So that means on Windows, we have to ignore any deltaX/Y value for zoom, and just use the magnitude. Also Chrome doesn't support high-precision touchpads on Windows. Firefox says that it does, but seems to have killed all inertia from the device.
There's a bunch of good questions here on the Chrome behavior, and unfortunately it was long enough ago that I don't have the answers off the top of my head. Big picture I implemented the behavior to be compatible with existing websites expectation for the meaning of wheel event deltas including Google maps, not AFAIK, the other way around. This did require a bit of math. See, for example, this comment: https://bugs.chromium.org/p/chromium/issues/detail?id=289887#c46. But I can believe there are some bugs / tweaks that should be made. Please feel free to file chromium bugs (https://crbug.com/wizard) for the current input team to look into. /cc dtpauska@chromium - current blink input lead
I collected several of the bugs and conversion in a fiddle and crbug here. Firefox has several serious issues: lack of pinch-zoom, lack of wheelDelta, numLinesToScroll on Windows thrown into deltaXY. I'll update this with more support for Firefox soon. https://jsfiddle.net/alecazam/my6gntrr/
This patch matches Chrome's behavior almost exactly, except it uses a slightly different formula for the deltaY value that results in the same movement speed as Chrome but doesn't drift over time. Tested by confirming that trackpad pinch-zoom now works on maps.google.com, wego.here.com, and figma.com.
Attachment #8474961 - Attachment is obsolete: true
Assignee: sshih → birunthan
Status: NEW → ASSIGNED
Comment on attachment 8865232 [details] [diff] [review] Map macOS pinch to zoom gesture to mousewheel with the control key Note that this assumes that official Firefox builds are built with the macOS 10.10 or earlier SDKs. If/when we target the 10.11 or later SDK, beginGestureWithEvent will no longer be called and we should instead check the event phase. See: https://developer.apple.com/reference/appkit/nsresponder/1526368-begingesturewithevent
Attachment #8865232 - Flags: review?(mstange)
Note that it's critical here to not fire a key-down for the fake ctrl-key, or at least set repeat on it so it can be skipped by key-down handlers. Typically the wheel event has to check if the event says ctrl is down, and the app has to check its keystate map for whether ctrl really is down. Do set ctrlKey when calling the wheel handler. On Windows, there's currently no easy way to detect user ctrl+wheel from fake ctrl for pinch, because the deriver sends the ctrl key and it looks just like the user pressing it. Finally you have to convert gesture to a wheel value exactly like Chrome does, so we can then convert it back. Typical values are delta = 0.2, and conversion is exp(somenum we use 4, -delta/100). These values have no correlation with real ctrl+wheel deltas from the mouse or touchpad wheel emulation. That's why it's critical to detect. You can compare to Safari gestures, but Chrome and Safari match values if you go through the conversions. It's just upsetting that there isn't gesture event support on desktop. It's so much easier and clearer for apps if pinch and zoom don't go through the same codepaths. For example, zoom may have an option to invert, but you always want pinch to work the same way.
I still think we really want this on all the platforms supporting pinch-to-zoom. Majority of users are on Windows after all. And inconsistency between platforms is bad.
Comment on attachment 8865232 [details] [diff] [review] Map macOS pinch to zoom gesture to mousewheel with the control key Review of attachment 8865232 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for the patch! Could you also add the code that's needed once we start building with the 10.11 or 10.12 SDK, and wrap it in an "#if !defined(MAC_OS_X_VERSION_10_11) || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_11"? The MAC_OS_X_VERSION_MAX_ALLOWED define reflects the SDK version. Are you interested in writing the Windows version of this patch or would you prefer somebody else to help out with that?
Windows Firefox/Edge/Chrome already do the pinch-zoom ctrlKey, so I'm not sure why you're asking for a Windows patch. The pinch zoom conversion must be done by the driver, but I haven't delved into the guts of Firefox. The issue is that the deltaY isn't really a gesture conversion like in Chrome on Mac. Also really need wheelDelta on Firefox, but that's a different topic.
(In reply to Markus Stange [:mstange] from comment #62) >Are you interested in writing the Windows version of this patch or would you prefer somebody else to help out with that? As mentioned by Alec, at least some touch pad drivers do send a WM_MOUSEWHEEL with the control key set, but maybe there is something more we can do compared to Chrome. Could we therefore land this patch for Mac and investigate Windows in a follow-up bug (I'm happy to do it before the next merge in June)? > Could you also add the code that's needed once we start building with the > 10.11 or 10.12 SDK, and wrap it in an "#if !defined(MAC_OS_X_VERSION_10_11) > || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_11"? Sure. (In reply to Olli Pettay [:smaug] from comment #61) > I still think we really want this on all the platforms supporting > pinch-to-zoom. > Majority of users are on Windows after all. > And inconsistency between platforms is bad. The majority of users expecting pinch to zoom to "just work" are on macOS. I imagine there are some users who switched to Chrome just because of this (even I am tempted when using e.g. Google Maps). The pinch to zoom story has been really bad on Windows. It has gotten better with the introduction of precision trackpads and Windows 10, but I will argue that most Windows users are not as accustomed to pinch zoom as their Mac counterparts. I agree that platform consistency is valuable in principle, but Firefox is strictly inferior to Chrome in this regard. Please also note that Chrome does not do pinch zoom any better than Firefox on Windows at the moment. So, for web compatibility, I will argue that landing this Mac only patchh is strictly better than not landing this at all. Also, is this really any worse than https://compat.spec.whatwg.org/?
Thanks, I wasn't really aware of the Windows situation. (In reply to Birunthan Mohanathas [:poiru] from comment #64) > I agree that platform consistency is valuable in principle, but Firefox is > strictly inferior to Chrome in this regard. Please also note that Chrome > does not do pinch zoom any better than Firefox on Windows at the moment. So, > for web compatibility, I will argue that landing this Mac only patchh is > strictly better than not landing this at all. I agree with this.
`beginGestureWithEvent` and `endGestureWithEvent` are not called for applications that link against the macOS 10.11 or later SDK when we're running on macOS 10.11 or later. For compatibility with all supported macOS versions, we have to call {begin,end}GestureWithEvent ourselves based on the event phase when we're using the 10.11+ SDK. See: https://developer.apple.com/reference/appkit/nsresponder/1526368-begingesturewithevent
Attachment #8866390 - Flags: review?(mstange)
This patch matches Chrome's behavior almost exactly, except it uses a slightly different formula for the deltaY value that results in the same movement speed as Chrome but doesn't drift over time. Tested by confirming that trackpad pinch-zoom now works on maps.google.com, wego.here.com, and figma.com.
Attachment #8866392 - Flags: review?(mstange)
Attachment #8865232 - Attachment is obsolete: true
Attachment #8865232 - Flags: review?(mstange)
`beginGestureWithEvent` and `endGestureWithEvent` are not called for applications that link against the macOS 10.11 or later SDK when we're running on macOS 10.11 or later. For compatibility with all supported macOS versions, we have to call {begin,end}GestureWithEvent ourselves based on the event phase when we're using the 10.11+ SDK. See: https://developer.apple.com/reference/appkit/nsresponder/1526368-begingesturewithevent
Attachment #8866393 - Flags: review?(mstange)
Attachment #8866390 - Attachment is obsolete: true
Attachment #8866390 - Flags: review?(mstange)
(In reply to Alecazam from comment #55) > Firefox exhibits numerous other problems on our Mac build. I rotate an > optical mouse wheel 10 times before one wheel event is sent to the app, so > some accumulator is not triggering off each notch. This doesn't happen on > Windows, and each notch sends a wheel event. I just re-read this comment from 1.5 months ago... this particular bug was bug 1355340 and should be fixed in Firefox 54.
Attachment #8866393 - Flags: review?(mstange) → review+
Comment on attachment 8866392 [details] [diff] [review] Part 2: Map macOS pinch to zoom gesture to mousewheel with the control key Review of attachment 8866392 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for the detailed comment.
Attachment #8866392 - Flags: review?(mstange) → review+
Pushed by birunthan@mohanathas.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1689fbd2179c Part 1: Fix gestures when using macOS 10.11 or later SDK. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/134875c28871 Part 2: Map macOS pinch zoom gesture to wheel event with control key. r=mstange
https://hg.mozilla.org/mozilla-central/rev/1689fbd2179c https://hg.mozilla.org/mozilla-central/rev/134875c28871
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Keywords: dev-doc-needed
I've documented this by adding a note to the browser compat sections on: https://developer.mozilla.org/en-US/docs/Web/API/WheelEvent https://developer.mozilla.org/en-US/docs/Web/Events/wheel I've also added a note to the Fx55 release notes: https://developer.mozilla.org/en-US/Firefox/Releases/55#DOM_HTML_DOM Can you let me know whether that covers it adequately, or is there more to be done? Thanks!
Keywords: dev-doc-needed → dev-doc-complete
Chris, looks great, thank you!
Depends on: 1399086
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: