Closed Bug 1721601 Opened 6 months ago Closed 3 months ago

Double tap with two fingers to zoom works on map websites such as google maps/bing maps while mousehover'ing the map itself

Categories

(Core :: Panning and Zooming, defect)

Firefox 91
Desktop
macOS
defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- wontfix
firefox91 + wontfix
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- wontfix
firefox95 --- fixed

People

(Reporter: dcicas, Assigned: tnikkel)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(4 files)

Affected versions

  • Fx 91.0b5
    Fx 92.0a2 (BuildID: 20210721092353 )

Affected platforms

  • macOS 11

Steps to reproduce

  1. Start Firefox.
  2. Reach google maps/bing maps.
  3. Double tap to zoom on the map.

Expected result

  • Double tap to zoom does not work on specific sites like google maps.

Actual result

  • You can double tap to zoom in on map websites.

Regression range

Has Regression Range: --- → yes
Has STR: --- → yes

Hello, Timothy!

It seems that Bug 1713589 might have caused this regression, what do you think?

Flags: needinfo?(tnikkel)

Thanks, I'll take a look.

Tracking for 91 as this is S2 and a regression.

Blocks: 674371
Flags: needinfo?(tnikkel)
Depends on: 1697766

We are relying on not finding anything to zoom in on in the maps case so that we don't zoom in. However bug 1713589 changed it so that we zoom in a small amount if we can't find anything to zoom in on, in order to provide feedback to the user.

Chrome also does this small zoom in behaviour, however they support preventDefault on a wheel listener to prevent a double tap from zooming (maps web apps preventdefault wheel events because they generally do their own zooming), we do not, this is tracked in bug 1697766.

We can:

  1. leave things as they are
  2. implement preventDefault for double taps
  3. backout bug 1713589 and no longer have this small zoom in behaviour.

Number 2) would require adding a bunch of fairly boilerplate code to apz, but I think it would be mostly straightforward.

Flags: needinfo?(tnikkel)

Thanks for the additional context, given that we are mid beta cycle, I think we should go with option 1 for 91, marking 91 as wontfix.

Flags: needinfo?(hikezoe.birchill)

What do we mean by double tap here?

  1. a double tap with two fingers
    this creates zoom in/zoom out of the full viewport
  2. a double tap with one finger.
    this zooms in the map without issues
    with Firefox 92.0a1 (2021-07-28) (64-bit) on macOS 11.5 (20G71) [edited the version]

So this works for me as expected. Or maybe I'm not testing the right thing.

(In reply to Karl Dubost💡 :karlcow from comment #6)

What do we mean by double tap here?

  1. a double tap with two fingers
    this creates zoom in/zoom out of the full viewport

That's not what we want to happen. We don't want any zooming to happen.

  1. a double tap with one finger.
    this zooms in the map without issues
    with Firefox 92.0a1 (2021-07-28) (64-bit) on macOS 11.5 (20G71) [edited the version]

That's what we want.

Attached image double-tap.jpg

OK about 1. Double tap with two fingers.

  • On Edge, it does nothing.
  • On Safari, it does nothing.
  • On Firefox, it zooms the viewport (not the map) This is a feature. It does the same thing for any articles. And it is working as expected.

For example, see the screenshot, before and after double tapping on the paragraph.
if I double tap again, it goes back to the full map.

That's cool. no?

ok so… indeed Dennis showed me the rationale wisdom. Indeed when on the map it should probably not do zoom at all. That makes sense.

Summary: Double tap to zoom works on map websites such as google maps/bing maps → Double tap with two fingers to zoom works on map websites such as google maps/bing maps while mousehover'ing the map itself

Yeah, on the map itself. We don't want to zoom then because then we get two forms of zoom going on which is confusing.

Found a Chromium issue that they send a wheel event.

There's an open webkit issue to provide a way to disable double tap zoom.

Hmm, dispatching wheel event sounds odd to me. Instead, Chromium guys should propose new event like beforezoom etc. wheel events whose all delta values are 0 may make web developers confused since it's hard to guess what the event means.

Usually, non-passive wheel event listeners is not used by documents (meaning non-web-apps). So, how about to stop the new feature of bug 1713589 if double tapped on elements whose ancestors have non-passive wheel event listeners for now?

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #13)

Usually, non-passive wheel event listeners is not used by documents (meaning non-web-apps). So, how about to stop the new feature of bug 1713589 if double tapped on elements whose ancestors have non-passive wheel event listeners for now?

It would fix this bug, but it feels a little "hacky" in that why would the presence of a non-passive wheel listener affect the behaviour (how much we decide to zoom) of a double tap, but not whether or not we do a double tap zoom at all?

I've assumed that he proposed disabling double tap zoom if there's any non-passive wheel event listeners?

Flags: needinfo?(hikezoe.birchill)

I'm not sure which one is better whether non-passive wheel event listeners affect to outside the double tapped elements or limited into the elements because 3rd party's script might add it to their own element.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #15)

I've assumed that he proposed disabling double tap zoom if there's any non-passive wheel event listeners?

Okay, if we are talking about disabling double tap zoom entirely when there are non-passive wheel listeners. That would disable double tap in cases where the page preventDefaults wheel events that have ctrl modifier, but it would also disable double tap for any non-passive wheel listener, even if it does nothing with ctrl+wheel events. We wouldn't send a potentiall confusing wheel event to content, but it's another special case.

Simon Fraser told me on the WebKit slack, he'd oppose the zoom event to prevent double tap zooming, to be precise he'd oppose the way to prevent by using preventDefault() in the event handlers, he suggested that a right way is using something similar to touch-action property. That sounds more promising to me.

Anyway, that would be a long-term solution.

(Note that Safari started firing wheel events on pinch zoom since this changeset, but doesn't fire on double tap zoom)

See Also: → 1735319

This also affects things like google sheets.

Flags: needinfo?(tnikkel)

Instead of passing this flag all of the time for double taps we want to decide to do this or not in CalculateRectToZoomTo. Moving it into ZoomTarget seems like a good way to do this (which is done in the next patch).

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED

With our current double tap zoom behaviour some sites like google maps or google sheets will zoom in when double tapping. This happens because of the behaviour added in bug 1713589. The goal of this change was to provide some user feedback (by zooming 1.2x) that they had double tapped when we couldn't find anything on the page to zoom in on. Chrome had this behaviour and it seemed desirable (Safari does not have this). The reason that Chrome does not zoom in on the above mentioned sites is because Chrome implements sending content a wheel event that it can preventDefault as a way to prevent the zoom from the double tap from happening (similar to pinch zooming) (Safari does not implement this, and we currently do not have this) and those sites do this preventDefaulting. That means we are the only browser that suffers from this problem.

The behaviour of sending a wheel event to be potentially preventDefaulted by content is not favoured by all browser vendors. Other solutions have been proposed, but movement on this does not seem imminent.

So this work around was proposed to fix this for now: disable the "zoom in if can't find something to zoom in on" behaviour if there is a non-passive wheel event listener on the target element or some ancestor.

Depends on D128435

Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9fa95cb8ad3a
Move "zoom in if can't zoom out" from being a flag to part of ZoomTarget for AsyncPanZoomController::ZoomToRect. r=botond
https://hg.mozilla.org/integration/autoland/rev/fc0a680bc208
If an element has a non-passive wheel listener don't do the "zoom in if can't zoom out" behaviour for macOS double tap zooms. r=botond
https://hg.mozilla.org/integration/autoland/rev/3e844ac540bd
Add test. r=botond

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

You need to log in before you can comment on or make changes to this bug.