1049014 - [B2G][E.me][Search Results] Long pressing on a search result launches the app when released.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat-Long-Press.txt

Description:
If the user Long Presses on a search result from the Rocketbar, a screen will open with an option to 'add to homescreen.'  If the user releases the long press, the app will launch, preventing the user from selecting the 'add to homescreen' option.


Repro Steps:
1) Update a Flame to 20140805000238
2) Tap on the Rocketbar and type 'social'
3) Long press the Twitter e.me app
4) Try to press the 'add to homescreen' option


Actual:
The app launches when the user releases the long press


Expected:
The app does not launch, allowing the user to select add to homescreen'

Environmental Variables:
Device: Flame 2.0
Build ID: 20140805000238
Gaia: 597975839c04e0198eb98c2c77474f057b5531e7
Gecko: ddeead927143
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Keywords: E.Me, Everything.Me, Rocketbar, Smart Search, Long Press, Homescreen


Repro frequency: 100%
See attached: video clip, logcat

Comment 1

[Martin Shuman [:Marty]]

This issue occurs on Flame 2.0 at both 512MB and 319MB

This issue does NOT occur on Flame 2.1 at either 512MB or 319MB.
The user is able to select 'Add to homescreen' normally.

Environmental Variables:
Device: Flame Master
Build ID: 20140805040204
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 7f81be7db528
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

[Peter Bylenga [:PBylenga]]

Qawanted for rest of the branch checks.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

Issue actually reproduces on Flame 2.1. It also reproduces on Buri 2.0.

Press and hold on an e.me search result brings up the Add to Homescreen option but the app launches itself on finger release, making it impossible to add the app to Homescreen in this method.

Device: Flame
Build ID: 20140805081353
Gaia: e93780f9da8b34f370a4113abd4df9780d58e443
Gecko: b50bb656674e
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Buri
Build ID: 20140805120616
Gaia: 327e0e70c97c12b0c28478266bc88eaf6882b4d1
Gecko: 4b9401dbb670
Version: 32.0 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

-----------
This issue does NOT reproduce on Flame 1.4.

Press and hold on an e.me search result brings up the Add to Homescreen option, and upon finger release, the option remains on screen and can be selected.

Device: Flame
Build ID: 20140805105735
Gaia: 9377274b17200a60cebcd2427d489a7756c4cc72
Gecko: 003ee6bd7777
Version: 30.0 (1.4)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 4

[Joshua Mitchell (Inactive)]

[Blocking Requested - why for this release]: regression, missing functionality (should have add to homescreen when long pressed)

Comment 5

[Jason Smith [:jsmith]]

Blocking per comment 4.

Comment 6

[Pi Wei Cheng [:piwei] (inactive)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
Build ID: 20140804131728
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 7f81be7db528
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken Environmental Variables:
Device: Flame
Build ID: 20140804133226
Gaia: f58bb4c40abbebd9ff9b32fb2a39bf3f44340991
Gecko: 2be403f035f5
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First broken gecko & last working gaia - issue does NOT repro
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 2be403f035f5

First broken gaia & last working gecko - issue DOES repro
Gaia: f58bb4c40abbebd9ff9b32fb2a39bf3f44340991
Gecko: 7f81be7db528

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/19bf9795263e2ccc15d824a52ebf23c2670fa9b9...f58bb4c40abbebd9ff9b32fb2a39bf3f44340991

Bug 1043293, or Bug 1041623, might be the cause?

Comment 7

[Joshua Mitchell (Inactive)]

broken by bug 1043293 ? Julien can you take a look?

Comment 8

[Julien Wajsberg [:julienw]]

It could definitely be the issue but I can't reproduce the issue on v2.1. May I know the difference between comment 1 and comment 3?

The 'search' application uses a real attribute 'contextmenu' to handle a context menu, I don't know how this plays with the event 'contextmenu'. If the event is not "defaultPrevented" then it could be the cause of the issue.

Comment 9

[Kevin Grandon :kgrandon]

I'll take it, I think I know of an easy fix.

Comment 10

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Here's a patch, I am working on a test.

Comment 11

[Kevin Grandon :kgrandon]

Comment on attachment 8468897 [details] [review]
Github pull request

Dale/Julien - Could either of you guys review this one? Thanks!

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8468897 [details] [review]
Github pull request

I couldn't reproduce the issue but the change looks fine. But maybe a marionette JS test could be a good idea so that we don't regress in the future?

I'm not sure that using the "contextmenu" attribute is actually a good idea here... maybe an idea is to preventDefault the event and _then_ send a "show" event to the menu itself? Not sure it's easier...

On a side note, I've also noticed that the contextmenu itself has "hidden", maybe it should have "type='popup'" instead, because in Firefox Desktop I couldn't make a "hidden" contextmenu appear, and so I wonder if we're doing the right thing here.

Comment 13

[Julien Wajsberg [:julienw]]

I'm keeping the r? to Dale in case he can reproduce the issue and so check the patch fixes it.

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8468897 [details] [review]
Github pull request

(In reply to Julien Wajsberg [:julienw] from comment #13)
> I'm keeping the r? to Dale in case he can reproduce the issue and so check
> the patch fixes it.

I think Dale is on PTO, so let's clear the review and let him comment when he's back. Thanks for the review, I agree about your concern of fixing 'contextmenu' but it seems a bit risky in 2.0.

Comment 15

[Kevin Grandon :kgrandon]

In Master: https://github.com/mozilla-b2g/gaia/commit/02b3534ea716cf6b598b0bf029d8444d92da2135https://github.com/mozilla-b2g/gaia/commit/02b3534ea716cf6b598b0bf029d8444d92da2135

Comment 16

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/7e207c9aef2b36692b8ae1f7ebdf02e84299aaed

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Reverted from v2.0 in https://github.com/mozilla-b2g/gaia/commit/7d92d1c2e378535acd2522fe3628b87034df48cd
for Gu bustage: https://tbpl.mozilla.org/php/getParsedLog.php?id=45487568&tree=Mozilla-B2g32-v2.0

Comment 18

[Kevin Grandon :kgrandon]

Attachment: v2.0 branch patch

Comment 19

[Kevin Grandon :kgrandon]

Landed a branch specific path with the 'Gu' fix: https://github.com/mozilla-b2g/gaia/commit/8d4599d18fbfc41f88ea494ab9cce0bb99cffac3

Comment 20

[Paladin]

This issue has been verified successfully on Flame 2.1

See attachment: Verify_video_Flame2.1.3gp
Reproducing rate: 0/5
Flame2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
--------------------------------------------------------------------------------------
This bug has been verified to fail on Flame 2.0
STR:
1) Tap on the Rocketbar and type 'social'
2) Long press the Twitter e.me app
**The device has no response.
Note:We can open the web while tap the Twitter icon.

See attachment: Verify_video_Flame2.0.3gp and logcat_flame2.0_1545.txt
Reproducing rate: 5/5
Flame 2.0 build:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141207000206
Version         32.0

Comment 21

[Paladin]

Attachment: Verify_video_Flame2.1.3gp

Comment 22

[Paladin]

Attachment: Verify_video_Flame2.0.3gp

Comment 23

[Paladin]

Attachment: logcat_flame2.0_1545.txt

Comment 24

[William Hsu [:whsu]]

Add NI?whsu to handle this bug.

Comment 25

[William Hsu [:whsu]]

Thanks everyone! :)
A follow up is submitted (Bug 1118676)