600846 - Multitouch: Starting zoom with bottom finger on Find in Page search field brings up Desktop Firefox input field context menu

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Aakash Desai [:aakashd]]

Attachment: bug screenshot

Build Id:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100930 Namoroka/4.0b7pre Fennec/4.0b1pre

Steps to Reproduce:
1. Go to www.mozilla.org
2. Click on the favicon to open the site panel
3. Click on Find in Page
4. Begin a multitouch with the bottom finger starting on the Find in Page search field
5. Move the fingers in until you see the Desktop Firefox input field context menu

Actual Results:
The context menu will appear where you released the bottom finger off the screen.

Expected Results:
The Desktop Firefox context menu should never pop-up

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

sounds like this related to bug 599066

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This is triggering a "click and hold" context menu.  Disabling ui.click_hold_context_menus fixes the bug, and we don't need that pref for content anymore.

We might want to re-enable this if we add context menus to chrome text fields in the future, e.g. for bug 585875.  If so then we might need a different fix for this bug.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

I'm glad we have a workaround, but I'm more interested to know why the problem happens at all. Why is a long tap happening?

Comment 4

[Wesley Johnston (:wesj)]

This isn't really a long-tap event in the Fennec sense. Its from:

http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#1754

which I think then fires the context menu from:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/textbox.xml#505

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Yeah, nsEventStateManager uses "ui.click_hold_context_menus" to fire the long-tap.

Wes, can you do a quick test to check what happens if we turn off "ui.click_hold_context_menus" and add a context menu to something in chrome? Does the Fennec "LongTap" kick in and fire something we can use to show a context menu?

Comment 6

[Wesley Johnston (:wesj)]

We can still pick up our own "TapLong" events, with or without the pref. Just have to add a listener. We can use those in chrome, and even use the context attribute if we want, to show the correct context menu. Is that what you're asking?

It seems silly to me that every XUL text field holds its own context menu.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #6)
> We can still pick up our own "TapLong" events, with or without the pref. Just
> have to add a listener. We can use those in chrome, and even use the context
> attribute if we want, to show the correct context menu. Is that what you're
> asking?

Yes, that is exactly what I wanted to know. If that is the case, I'll r+ the patch.

Comment 8

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/9cb5e452f73c

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on build:

Mozilla/5.0 (Android; Linux armv71; rv:2.0b7pre) Gecko/20101008 Namoroka/4.0b7pre Fennec/4.0b2pre

Comment 10

[Matt Brubeck (:mbrubeck)]

Backed out because of bug 602868:
http://hg.mozilla.org/mobile-browser/rev/d603ff51886a

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Remove context menu

Sorry to steal from you Matt. This just kills these two bindings. textbox-input-box only has things dealing with the context menu as far as I can see. The spell check one only has things dealing with the context menu and spell check, which we don't use. So I just killed them both.

Alternatively, we could put in the effort to make menupopups work.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 482370 [details] [diff] [review]
Remove context menu

creative approach

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/28bc43358a35

Comment 14

[Aakash Desai [:aakashd]]

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=13672 created to regression test this bug.

Comment 15

[Aakash Desai [:aakashd]]

Mozilla/5.0 (android Linux armv7l; rv:2.0b7pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2