640500 - Long tap on a website from Awesomelist doesn't open the context menu

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Cristian Nicolae (:xti)]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Build Identifier: 

A website can not be opened in a new tab or shared because a long tap on it will not display the context menu.

Reproducible: Sometimes

Steps to Reproduce:
1. Open Fennec App
2. Do some browsing
3. Tap on URL Bar
4. Long tap on any browsed website

Actual Results:  
The context menu is not shown. When the long tap is released over the website, it will open.

Expected Results:  
Context menu appears and the user is able to choose to open it in a new tab or share that link.

The issue is reproducing 8 times from 10 on Sony Ericsson Xperia X10 - Android 2.1-update1

Build id : Mozilla/5.0 (Maemo;Linux armv7l;rv:2.0b13pre)Gecko/20110309
Firefox/4.0b13pre Fennec /4.0b6pre
Device: Sony Ericsson Xperia X10
OS: Android 2.1 update 1

Comment 1

[Cristian Nicolae (:xti)]

Attachment: Long tap log

Comment 2

[Cristian Nicolae (:xti)]

Please see the following video: http://www.youtube.com/watch?v=T2F74p1uS4s&feature=channel_video_title

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

The patch moves from contextmenu to TapLong since we used it pretty much anywhere in the file (sigh).

Comment 4

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

Comment on attachment 518381 [details] [diff] [review]
Patch

Do we know why "contextmenu" is not good enough?

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #4)
> Comment on attachment 518381 [details] [diff] [review]
> Patch
> 
> Do we know why "contextmenu" is not good enough?

Honestly no :/.
It looks like it is not fired sometimes.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 518381 [details] [diff] [review]
> > Patch
> > 
> > Do we know why "contextmenu" is not good enough?
> 
> Honestly no :/.
> It looks like it is not fired sometimes.

hmm, I do wonder if that a possible regression from http://hg.mozilla.org/mobile-browser/rev/be135f4d1a63 since ui.dragThresholdX/Y are used by http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#2010 to determine if it is a drag and in this case cancel the contextmenu event.

It defaults to 5 if nothing is set and with the big fat fingers we have...

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Ok, this makes like much better.

Asking r? to Matt to check I'm not breaking something with this change.

Comment 8

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

this patch looks like the patch on bug 640613 ?

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #8)
> this patch looks like the patch on bug 640613 ?

Obviously, I've discovered the problem on bug 640613 by trying to reproduce this bug! So both fixes lives in the same area of code...

If needed I could merge them but I guess bug 640613 is less important than this one so if it has bad effect we could ignore it for the release.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch - the right one!

Sorry for the confusion, it was the wrong patch :(

Comment 11

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

Comment on attachment 518726 [details] [diff] [review]
Patch - the right one!

Looks OK to me, but getting a second opinion

Comment 12

[Matt Brubeck (:mbrubeck)]

Comment on attachment 518726 [details] [diff] [review]
Patch - the right one!

Good catch - sorry for the regression!

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/729dd25267df

Comment 14

[Anna (Waverley)]

VERIFIED FIXED on
 
Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre Fennec /4.0b6pre 

Device: Sony Ericsson Xperia X10 (Android 2.1 update 1)