1198522 - Long pressing on an image in edit mode will bring up a menu that does not function

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Runtime, defect)

Description

[Adam Alldredge [:AdamA][Inactive]]

Attachment: logcat

Description:
When in edit mode if the user long presses on the image it will bring up a menu with the options "Copy Image" "Save Image" and "Share Image." these options do not work correctly.

Repro Steps:
1) Update a Aries to 20150825022022
2) Open Contacts app
3) choose to create a new contact
4) Long press on the circle for the image
5) Select "Save Image"
6) Observe results

Actual:
The menu options do not work as intended

Expected:
It is expected that the menu options work as intended

Environmental Variables:
Device: Aries 2.5 [Full Flash]
Build ID: 20150825022022
Gaia: b441bde54293bea5254dc340845effe951fa3906
Gecko: 04b8c412d9f5
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Repro frequency: 10/10
See attached: video clip(https://youtu.be/59jFYI9Wv1c), logcat

Comment 1

[Adam Alldredge [:AdamA][Inactive]]

This issue DOES occur on Flame 2.5.

Environmental Variables:
Device: Flame 2.5 [Full Flash]
Build ID: 20150825064342
Gaia: b441bde54293bea5254dc340845effe951fa3906
Gecko: f3df9cd1701f
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Result:
The menu options do not work as intended
----------------------------
This issue DOES NOT occur on Flame 2.2

Environmental Variables:
Device: Flame 2.2 [Full Flash
Build ID: 20150825032504
Gaia: 335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gecko: 1effc4cb6414
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Result:
I different menu appears when you long press on the image.

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

[Blocking Requested - why for this release]:

Non functional menu so nominating this to block.

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

Johan, can you take a look at this please?

Comment 4

[Johan Lorenzo [:jlorenzo] - PTO - Back April 13th]

Comms triage: We have the feeling the entire activity selector is not the right one. Adam could you attach a screenshot of the menu in 2.2?

Comment 5

[Mahendra Potharaju [:mahe]]

Blocking for 2.5. Non functional menu

Comment 6

[Adam Alldredge [:AdamA][Inactive]]

Attachment: 2.2 Long press menu

Attaching an image of the menu from Flame 2.2.

Comment 7

[Johan Lorenzo [:jlorenzo] - PTO - Back April 13th]

IMO, The menu should remain as the same in 2.2. This is a regression.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1198522.contactsphotomenu > mozilla-b2g:master

Comment 9

[Borja Salguero [:borjasalguero]]

Attachment: Long press menu

After testing master I've seen that there is another context menu (and it seems to be a common element in System, because I can reproduce the same in multiple apps).

This new UI module it's buggy, due to it should not appear when the photo of a contact it's empty...

Adam, could you double check this? I think that there is a bug in here colliding with the solution added by Fernando. Thanks!

Comment 10

[Borja Salguero [:borjasalguero]]

Comment on attachment 8653499 [details] [review]
[gaia] ferjm:bug1198522.contactsphotomenu > mozilla-b2g:master

Removing the 'review' flag due to it seems that it's other bug involved. Please ask me to review this if needed once we have the whole info from QA. Thanks!

Comment 11

[Adam Alldredge [:AdamA][Inactive]]

Yes, on the latest master there is a context menu that is appearing when long pressing on the contacts image with no picture.

Environmental Variables:
Device: Aries 2.5 [Full Flash]
BuildID: 20150901190824
Gaia: c2582f4be03cd12124b96a263c8d14c774f0ffe4
Gecko: e47423c01964
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 12

[Maria Angeles Oteo (:oteo)]

After checking the behaviour in 2.2 in Contacts and that should be the correct one:
- When long-pressing on the circle and there is not any picture yet, the menu offers "Camera" and "Gallery" options (following the STR reported in the bug)

- When there is already a picture attached in the Contact, when long-pressing on it, the action menu shows "Remove photo" and "Change photo" options. That's the screenshot attached in Comment 6.

Right now in master, for both cases (with and without photo), when long pressing on the image circle, an action menu is shown with "Copy image", "Save image" and "Share image" which does not make sense.


Apart of that, as Borja explains, this issue can be reproduced in other applications (e.g. doing long press on a photo taken inside Camera application, in the preview). For that reason, setting regressionwindow-wanted to identify the cause of the regression and move the bug to the correct component.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

I guess this was caused by bug 952456. Boris, could you give us some pointers, please? Thanks!

Comment 14

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

b2g-inbound regression window:

Last Working
Device: Flame
BuildID: 20150821074538
Gaia: 87dfe24140394245b45d4e7afce81006ad7ef399
Gecko: ffcbb3d4e7a3c15e6f7ef6a1e64359eb40d39529
Version: 43.0a1 (2.5 Master) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

First Broken
Device: Flame
BuildID: 20150821080238
Gaia: ce2a1760b1d130cb24b53080e67dddf8b68b9de2
Gecko: 993235ea08c7ec40b87b38abfec19288ea99f1f3
Version: 43.0a1 (2.5 Master) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Last Working Gaia First Broken Gecko - repro
Gaia: 87dfe24140394245b45d4e7afce81006ad7ef399
Gecko: 993235ea08c7ec40b87b38abfec19288ea99f1f3

Last Working Gecko First Broken Gaia - no repro
Gaia: ce2a1760b1d130cb24b53080e67dddf8b68b9de2
Gecko: ffcbb3d4e7a3c15e6f7ef6a1e64359eb40d39529

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=ffcbb3d4e7a3c15e6f7ef6a1e64359eb40d39529&tochange=993235ea08c7ec40b87b38abfec19288ea99f1f3

Confirmed the bug mentioned at comment 13 is the caused. Caused by Bug 952456.

Comment 15

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Ni? is on Boris already.

Comment 16

[Boris Chiou [:boris]]

I know the root cause:
Bug 1029336 disabled the context menu for certified apps
(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_context_menu.js#L52)
, but Bug 952456 enabled it accidentally.

Bug 952456 added a "copy image" option in detail.contextmenu, but this option should be a system menu option, not a customized one, so I think we should revise the if-condition to make sure only customized context menu items are checked.

Comment 17

[Boris Chiou [:boris]]

Attachment: Separate customized and sys cmd menu items

Use a different array to store 'copy-image' and 'copy-link' menu items.

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] BorisChiou:Bug1198522 > mozilla-b2g:master

Comment 19

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=329c59573e9d

Comment 20

[Boris Chiou [:boris]]

HI ferjm,
Could you please double check my patches? It should disable the redundant menu when you long press the image in edit mode. Thanks.

Comment 21

[Boris Chiou [:boris]]

Attachment: Separate customized and sys cmd menu items (v2, carry kanru's r+)

Use a different array to store 'copy-image' and 'copy-link' menu items.

Fix mochitest.

Comment 22

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3f8e5b69c33

Comment 23

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8656434 [details] [review]
[gaia] BorisChiou:Bug1198522 > mozilla-b2g:master

browserElement_ContextmenuEvents.js needs update -- it should assert not only the # of items but also the content of the objects.

Comment 24

[Boris Chiou [:boris]]

Attachment: Separate customized and sys cmd menu items (v3, carry kanru's r+)

Use a different array to store 'copy-image' and 'copy-link' menu items.

Add more mochitests to test the content of menu objects.

Comment 25

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e254ae1818e2

Comment 26

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b6975c0db3a

Comment 27

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9f65a1129f3

Comment 28

[Boris Chiou [:boris]]

Hi, Sheriff
Could you please merge my gaia patch and gecko patch together? Thanks.

Comment 29

[Boris Chiou [:boris]]

(In reply to Maria Angeles Oteo (:oteo) from comment #12)
> After checking the behaviour in 2.2 in Contacts and that should be the
> correct one:
> - When long-pressing on the circle and there is not any picture yet, the
> menu offers "Camera" and "Gallery" options (following the STR reported in
> the bug)
> 
> - When there is already a picture attached in the Contact, when
> long-pressing on it, the action menu shows "Remove photo" and "Change photo"
> options. That's the screenshot attached in Comment 6.
> 
> Right now in master, for both cases (with and without photo), when long
> pressing on the image circle, an action menu is shown with "Copy image",
> "Save image" and "Share image" which does not make sense.
> 
> 
> Apart of that, as Borja explains, this issue can be reproduced in other
> applications (e.g. doing long press on a photo taken inside Camera
> application, in the preview). For that reason, setting
> regressionwindow-wanted to identify the cause of the regression and move the
> bug to the correct component.

On my patches, I disable the context menu with copy/save/share image options, and the behavior in master should be the same as that in 2.2 after we merge these patches. Thanks for your reproduction and check.

Comment 30

[Boris Chiou [:boris]]

(In reply to Maria Angeles Oteo (:oteo) from comment #12)
> After checking the behaviour in 2.2 in Contacts and that should be the
> correct one:
> - When long-pressing on the circle and there is not any picture yet, the
> menu offers "Camera" and "Gallery" options (following the STR reported in
> the bug)
> 
> - When there is already a picture attached in the Contact, when
> long-pressing on it, the action menu shows "Remove photo" and "Change photo"
> options. That's the screenshot attached in Comment 6.
> 
> Right now in master, for both cases (with and without photo), when long
> pressing on the image circle, an action menu is shown with "Copy image",
> "Save image" and "Share image" which does not make sense.
> 
> 
> Apart of that, as Borja explains, this issue can be reproduced in other
> applications (e.g. doing long press on a photo taken inside Camera
> application, in the preview). For that reason, setting
> regressionwindow-wanted to identify the cause of the regression and move the
> bug to the correct component.

The context menu with copy/save/share image options shouldn't be displayed, and the behavior in master branch should be the same as that in 2.2 after we merge these patches.
Thanks for your reproduction and check.

Comment 32

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/b0c1649e39d2

Comment 33

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/676caf58c4de93ba0d6a93add4d0a2ebfaa67764 for the gaia part

Comment 34

[Carsten Book [:Tomcat]]

sorry had to revert this from gaia and b2g-inbound for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=2731483&repo=b2g-inbound

Comment 35

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/b2g-inbound/rev/bd613b5b2e23

Comment 36

[Boris Chiou [:boris]]

(In reply to Pulsebot from comment #35)
> Backout:
> https://hg.mozilla.org/integration/b2g-inbound/rev/bd613b5b2e23

Thanks, but comment 25 didn't see the crash and I cannot reproduce this on my mac. I will take a look at this problem on try server.

Comment 37

[GitHub Autolander Bot]

Attachment: [gaia] BorisChiou:Bug1198522 > mozilla-b2g:master

Comment 38

[Boris Chiou [:boris]]

Comment on attachment 8660245 [details] [review]
[gaia] BorisChiou:Bug1198522 > mozilla-b2g:master

Carry Tim's r+. Just fix try server errors.

Comment 39

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2233d73dd603

Comment 40

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=15f753a73e4c

Comment 41

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6491e543805f

Comment 42

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f5b307031cd

Comment 43

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93d022df2a27

Comment 44

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=88ad83da2967

Comment 45

[Boris Chiou [:boris]]

Attachment: Add a flag to make sure customized menu is added (v4)

We use the same array to maintain some system items and customized menu
items, so I add a new flag to make sure we add customized ones.

Note: I changed the implementation to avoid gij20 test failure on b2g-desktop (linux64)

Comment 46

[Boris Chiou [:boris]]

Attachment: Add a flag to make sure customized menu is added (v5)

We use the same array to maintain some system items and customized menu
items, so I add a new flag to make sure we add customized ones.

Comment 47

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7af3a16f7b1a

Comment 48

[Boris Chiou [:boris]]

Comment on attachment 8662919 [details] [diff] [review]
Add a flag to make sure customized menu is added (v5)

Review of attachment 8662919 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Kanru,
I need your review again because I changed the implementation to avoid the gij20 failure on b2g-desktop linux64. (However, my original patches can pass all gij tests on Mulet linux64.) I don't know the root cause because I cannot reproduce it on my local linux & mac. All I know is that this problem happened while it traversed the second array.

The new patch uses only one array to handle this menu items to avoid the gij20 failure.
Thanks.

Comment 49

[Boris Chiou [:boris]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dcce578aa06

Comment 50

[Boris Chiou [:boris]]

(In reply to Boris Chiou [:boris] from comment #49)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dcce578aa06

All gij and gu tests are passed. Good news!

Comment 51

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/574e834c0198

Comment 52

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

https://hg.mozilla.org/mozilla-central/rev/574e834c0198

Comment 53

[Boris Chiou [:boris]]

Hi Tim,

Gecko part was merged. Could you please merge the gaia PR? Thanks.

Comment 54

[Tim Guan-tin Chien [:timdream] (please needinfo)]

master: https://github.com/mozilla-b2g/gaia/commit/98bc7efb7c79b180bde067b3a1b000106c356f13

If you split Gaia/Gecko into two bugs, releng will help you land Gaia patches. Alternatively I could just give you GitHub permission... :)

Comment 55

[Maria Angeles Oteo (:oteo)]

Tested in latest master build (9/24) and working fine. Thanks Boris!

Environmental Variables (9/24):
flame master (2.5 version)
Build ID: 20150924072151
Gecko: 65a0dd9
Gaia: 7e72976
Platform version: 44.0a1

Comment 56

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: Long pressing on an image in contact edit mode will bring up two menus: "Copy Image/Save Image/Share Image" and "Camera/Gallery" (or "Remove photo/Change photo"). But the "Copy Image" and "Save Image" option does not function at present.

See attachment: verified_Aries_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20150924150202
Gaia Revision          4bb17b24620818cbda0ba0c0d69e0ce3f914e1b7
Gaia Date              2015-09-23 16:06:39
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/001942e4617b2324bfa6cdfb1155581cbc3f0cc4
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150924.183612
Firmware Date          Thu Sep 24 18:36:29 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20150925003138
Gaia Revision          4bb17b24620818cbda0ba0c0d69e0ce3f914e1b7
Gaia Date              2015-09-23 16:06:39
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/eee4266046984718e4daa99d94ce820f3fd86d32
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150925.001452
Firmware Date          Fri Sep 25 00:15:01 UTC 2015
Bootloader             s1

Comment 57

[Shally Li（leave from Mozilla）]

Attachment: verified_Aries_v2.5.3gp

Comment 59

[Chris Mills [:cmills]]

I have mentioned this bug in a new section in the 2.5 release notes:

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Releases/2.5#Bugs_and_regressions

Is this ok?