pfield Reporter
	Description • 11 years ago

Description: Attempting to long press on an attached, but not sent MMS file, will result in the 'add attachment menu' to appear and overlap over the media interaction menu.

Repro Steps:
1) Updated Buri to Build ID: 20131120004000
2) Enter into messaging app and attach a media file.(E.G. Music file)
3) Long press the media file to view the media interaction menu  (E.G "listen, Remove Audio, Replace Audio.")

Actual:
Menu to 'add attachment' appears when not activated and overlaps appropriate media interaction menu.

Expected:
The 'add attachment menu' does not appear when not activated, and does not overlap other menus.

Environmental Variables
Device: BURI v 1.2.0 COM RIL
Build ID: 20131120004000
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/2d454e0de2ed
Gaia: 5ec2963fff60492c840707df8d8090f9908a5251
Platform Version: 26.0
RIL Version: 01.02.00.019.102 

Notes:
Repro frequency: 2/3

See attached: logcat
Closing the overlapping 'add attachment menu' will show the correct menu.

	Jason Smith [:jsmith]
	Comment 1 • 11 years ago

1. This needs a screenshot
2. Can someone check if this reproduces on 1.1?

	Julien Wajsberg [:julienw]
	Comment 2 • 11 years ago

We probably miss a stopPropagation in compose.js line 422.

Probably a regression from bug 923023, but comes from a code that was there before.

Fernando, could you take it?

	Fernando Campo (:fcampo)
	Comment 3 • 11 years ago

Sure, taking a look asap

	Jason Smith [:jsmith]
	Comment 4 • 11 years ago

(In reply to Julien Wajsberg [:julienw] from comment #2)
> We probably miss a stopPropagation in compose.js line 422.
> 
> Probably a regression from bug 923023, but comes from a code that was there
> before.
> 
> Fernando, could you take it?

I don't think this is a regression from bug 923023 - the bug as filed here targets 1.2, where as bug 923023 only targets 1.3.

	Sarah Parsons
	Comment 5 • 11 years ago

A screenshot would provide no information, please see video the reporter made now attached. 

This issue does not reproduce on the Leo 1.1 Build ID: 20131120041201

Gaia   b585b32441fafa67f2b4582db23be5f3a2afab21
SourceStamp a9fa9a04390d
BuildID 20131120041201
Version 18.0

This issue reproduces all the back to the first Buri 1.2 Build ID:20130621031231 and can be reproduced subsequently on every 1.2  build.

Gaia   e2f19420fa6a26c4287588701efaec09a750dba1
SourceStamp 7ba8c86f1a56
BuildID 20130621031231
Version 24.0a1

	Jason Smith [:jsmith]
	Comment 7 • 11 years ago

Hmm...this is a minor regression, looks like it would be easy workaround this.

	Julien Wajsberg [:julienw]
	Comment 8 • 11 years ago

"workaround" is not the good word, it's probably very easy to fix for real, see comment 2.

You're right about Bug 923023 it's likely not the good culprit.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 9 • 11 years ago

triage: v1.3+ regresion

	Fernando Campo (:fcampo)
	Comment 10 • 11 years ago

I've discovered that this is just understanding a long press as a double click. If you make the long press on the attachment a little lower than in the video (which is right the position where 'Replace' option will appear), instead of the new menu, the window disappear (you just pressed 'Cancel' option).

Also, I can repro on master, but I'm not sure on how to handle it, as it is not differenciating a long press

	Julien Wajsberg [:julienw]
	Comment 11 • 11 years ago

Just try my comment 2 ;) And maybe we miss a preventDefault somewhere too.

If you preventDefault a longpress, it doesn't do a click.

	Julien Wajsberg [:julienw]
	Comment 12 • 11 years ago

Since Fernando is in holiday, reseting the assignee so that someone else can take it.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 13 • 11 years ago

to steve for now

	Steve Chung [:steveck] Assignee
	Comment 14 • 11 years ago

Hi Julien, besed on the discussion with you this morning, we think the context menu event listener is ok to be removed here and also fix the bug. This patch only removed the context menu listener and the menu will not show up befor finger released now.

	Julien Wajsberg [:julienw]
	Comment 15 • 11 years ago

Comment on attachment 8356091 [details]
Link to github

r=me

thanks !

	Julien Wajsberg [:julienw]
	Comment 16 • 11 years ago

FTR I tried to add a unit test sending the "contextmenu" event to see if the "click" event was generated, but the "contextmenu -> click" behavior seems to come from elsewhere in gecko.

	Steve Chung [:steveck] Assignee
	Comment 17 • 11 years ago

Landed in master: dfd7958910ee1e70f72d140e250c25cd7ea286f8

I've tried to create a test case for it, but I was unable to mock and dispatch a contextmenu event in this case... :/

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 18 • 11 years ago

Uplifted dfd7958910ee1e70f72d140e250c25cd7ea286f8 to:
v1.3: 05680a7bcfdb1a71b407c05746c885bde0eb4c7e