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

Mousewheel zoom broken and resolution constraints not respected in Firefox #1789

Merged
merged 1 commit into from Mar 13, 2014

Conversation

ahocevar
Copy link
Member

I was unable to figure out what is wrong, but this issue can be easily reproduced. It only happens in Firefox:

  1. Open http://ol3js.org/en/master/examples/simple.html
  2. Try to mousweheel-zoom in. Only works for me after several attempts, and only after mousewheel-zooming out.
  3. Open the console and check map.getView().getView2D().getZoom(). You are very likely to get undefined, which means we have an unconstrained resolution.

Any ideas?

@twpayne
Copy link
Contributor

twpayne commented Mar 2, 2014

#436 and #843 report the same problem.

Mac OS X seems to create its own mouse wheel zoom events when using the trackpad. The trackpad can easily generate tens of mouse wheel events. However, zooming with a real mouse wheel only generates a single event.

I suspect that Chrome and Safari look at the device generating the mouse wheel events and handle the trackpad and the mousewheel differently, but that Firefox does not. However, it should not be possible to end up on an unconstrained resolution, so there's a bug in ol3 too.

@twpayne
Copy link
Contributor

twpayne commented Mar 3, 2014

Note that this seems specific to Firefox on Mac OS X. I'm unable to reproduce the problem on Firefox 27 on Ubuntu.

@bartvde
Copy link
Member

bartvde commented Mar 3, 2014

@ahocevar
Copy link
Member Author

ahocevar commented Mar 3, 2014

So browser issue aside, I almost got crazy looking to see why the constraints are ignored, and I do not have the slightest clue.

@twpayne
Copy link
Contributor

twpayne commented Mar 3, 2014

I agree, I have no idea why the constraints are being ignored. I'll try some debugging when my Mac comes back from repair...

@ahocevar
Copy link
Member Author

ahocevar commented Mar 3, 2014

Thanks @twpayne, much appreciated!

@ahocevar
Copy link
Member Author

According to this thread on the mailing list, the ignored constraints are an issue not specific to Firefox.

@elemoine
Copy link
Member

If you do map.getView().getResolution() in the console do you effectively get an unconstrained resolution? I'm trying to determine if the bug lies in the interaction or in the getZoom function.

I tried to reproduce the getZoom return undefined issue in FireFox/Linux but I couldn't.

@ahocevar
Copy link
Member Author

Yes, map.getView().getResolution() returns an unconstrained resolution. The only browser I was able to reproduce this in was Firefox (OSX), with trackpad. I tried to debug, but didn't find any hint. Note that the issue is also present in the pointerevents branch. So I guess it is something with the mousewheelzoom interaction not finishing a zoom operation.

@elemoine
Copy link
Member

The weird part it that the mousewheel interaction always zooms the view to constrained resolutions. See https://github.com/openlayers/ol3/blob/master/src/ol/interaction/interaction.js#L175-177.

@ahocevar
Copy link
Member Author

It looks like goog.events.MouseWheelHandler normalizes the delta to integer values, but we divide the value by 3. The default constraint adjusts the zoom level based on the delta. If passed a non-integer value, the result will be a fractional zoom level.

I see basically two ways to handle this: either change the constraint function to round the delta, or to change the mousewheel code to not divide the delta by 3. I'm attaching a pull request that does the latter, because I do not notice a change in behavior in other browsers (as opposed to rounding the delta before calling the constraint function).

@ahocevar
Copy link
Member Author

Pull request attached.

@twpayne
Copy link
Contributor

twpayne commented Mar 13, 2014

Thanks very much for investigating this @ahocevar! This patch seems to fix the problem on Firefox on Mac OS X. It does make zooming very fast though, but I'd suggest addressing that in a different PR since this fixes a significant bug.

@elemoine
Copy link
Member

+1

@ahocevar
Copy link
Member Author

Thanks for the reviews. I think it will make sense to revisit this when the pointerevents branch is merged. It might make sense to get rid of the goog.events.MouseWheelHandler abstraction and run our own.

ahocevar added a commit that referenced this pull request Mar 13, 2014
Mousewheel zoom broken and resolution constraints not respected in Firefox
@ahocevar ahocevar merged commit 1a40b17 into openlayers:master Mar 13, 2014
@ahocevar ahocevar deleted the mousewheel-delta branch March 13, 2014 22:43
@ahocevar
Copy link
Member Author

@elemoine:

The weird part it that the mousewheel interaction always zooms the view to constrained resolutions.

Yes, but the constraint used allows fractional zoom levels when the delta is not an integer. It's more a strategy for calculating a zoom level than a constraint. See https://github.com/openlayers/ol3/blob/master/src/ol/resolutionconstraint.js#L65-70.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants