1653576 - [touchpad] Zooming on dropdown lists/context menus using a touchpad closes the dropdown

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Hani Yacoub, Desktop QA]

Affected versions

Firefox Nightly 80.0a1

Affected platforms

Windows 10 x64

Preconditions

fx.webrender.all = true
apz.allow_zooming = true
apz.windows.use_direct_manipulation = true

Steps to reproduce

1. Navigate to a page which contains dropdown lists (e.g.https://twitter.com/i/flow/signup)
2. Open a dropdown list(e.g. Month/Day)
3. Zoom in using a touchpad

Expected result

APZ zooming is performed on the list

Actual Result

Zooming on dropdown lists/context menus using a touchpad closes the dropdown

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Sounds like a dupe of bug 1647546.

Comment 2

[Hani Yacoub, Desktop QA]

I was aware of bug 1647546, but it seems to be referring to hamburger menu or other Firefox menus.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Yeah I'm not 100% sure what all is covered by bug 1647546 either but it's likely this bug has the same root cause if it's not an exact dupe.

Comment 4

[Timothy Nikkel (:tnikkel)]

I think this is different than bug 1647546.

A select dropdown is a separate OS window. If the user pinch zooms while a select drop down is open we can't smoothly update the position of this OS window. We could:

1. ignore pinch zooms while a select drop down is open
2. close the select drop down and pinch zoom the page
3. update the position of the select drop down every 500 ms or some other time period.

If the cursor is over the select drop down itself I don't think we want to bother supporting zooming just the select drop down itself.

We currently do 2).

If the cursor is over the select drop down itself it might make more sense to just ignore the pinch zoom rather than close the select drop down.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I can look at this.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

A select dropdown is a separate OS window. If the user pinch zooms while a select drop down is open we can't smoothly update the position of this OS window. We could:

1. ignore pinch zooms while a select drop down is open
2. close the select drop down and pinch zoom the page
3. update the position of the select drop down every 500 ms or some other time period.

If the cursor is over the select drop down itself I don't think we want to bother supporting zooming just the select drop down itself.

We currently do 2).

If the cursor is over the select drop down itself it might make more sense to just ignore the pinch zoom rather than close the select drop down.

Yeah. Chrome and Edge both do (1), and I think we should do the same. I'll see if I can figure out how to do that.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Note also that attempting a two-finger trackpad scroll on a select dropdown closes the dropdown instead of scrolling it, which I think is pretty undesirable. Chrome and Edge both scroll it.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Backing out bug 1636061 fixes this bug and bug 1647546, although trackpad pinch gestures seem to trigger scrolling. While that's arguably undesirable, it's better than closing the popup.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1653576 - Don't let dmanip events roll up popups if the events are inside the popup. r?tnikkel

The existing rollup-handling code in SendAnAPZEvent was mostly copied from
the DealWithPopups function, so this patch removes that duplicate and reuses
the existing codepath. And it adds some extra checking for dmanip events that
mirrors the checking we do for other events, so that events that are targeted
inside the popup don't cause the popup to get rolled up.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=5524844d8a7c5f51d682b0e071b9a996f8b65e15 has some aarch64 windows builds (what I needed for testing on my lenovo).

Comment 11

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)

Backing out bug 1636061 fixes this bug and bug 1647546

Yeah, then we need a solution for select dropdowns detaching from their anchor content when scrolling or zooming. That solution can be "just live with it (at least for now)".

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1653576 - Generify EventIsInsideWindow to allow passing a point. r?tnikkel

This allows us to reuse this function and the GetPopupsToRollup function when
an event point is different than what comes out of GetMessagePos.

For instance, the WM_POINTERDOWN code can now reuse this functions properly.
This fixes a latent bug where it was calling GetPopupsToRollup which was using
an possibly-incorrect point internally.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15b0c4b2dbe8
Generify EventIsInsideWindow to allow passing a point. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/510e2a73c290
Don't let dmanip events roll up popups if the events are inside the popup. r=tnikkel

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/15b0c4b2dbe8
https://hg.mozilla.org/mozilla-central/rev/510e2a73c290

Comment 16

[Hani Yacoub, Desktop QA]

I confirm that pinch zooms doesn't close drop downs while a select drop down is open on Firefox Nightly 82.0a1 (2020-09-03) but comparing with Chrome where pinch zooms are totally ignored on drop downs on Firefox it's scrolls through the drop down.

Is this the desirable behaviour in this case?

Comment 17

[Kartikaya Gupta (email:[e-mail])]

No, it seems like we should be ignoring it, the same as Chrome. Can you please file a new bug for that?

Comment 18

[Hani Yacoub, Desktop QA]

Verified as fixed on the latest Firefox Nightly 82.0a1. Logged bug 1663646 so pinch zooms on dropdown to be totally ignored.

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Thanks!