Closed Bug 1198522 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox OS Graveyard :: Runtime, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.5+, firefox44 fixed, b2g-v2.2 unaffected, b2g-master verified)

VERIFIED FIXED
FxOS-S8 (02Oct)
blocking-b2g 2.5+
Tracking Status
firefox44 --- fixed
b2g-v2.2 --- unaffected
b2g-master --- verified

People

(Reporter: AdamA, Assigned: boris)

References

()

Details

(Keywords: dev-doc-complete, regression, Whiteboard: [2.5-Daily-Testing][Spark])

Attachments

(7 files, 5 obsolete files)

347.23 KB, text/plain
Details
20.97 KB, image/png
Details
46 bytes, text/x-github-pull-request
Details | Review
16.32 KB, image/png
Details
46 bytes, text/x-github-pull-request
timdream
: review+
Details | Review
6.90 KB, patch
kanru
: review+
Details | Diff | Splinter Review
3.58 MB, video/3gpp
Details
Attached file 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
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.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [2.5-Daily-Testing][Spark]
[Blocking Requested - why for this release]:

Non functional menu so nominating this to block.
blocking-b2g: --- → 2.5?
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Johan, can you take a look at this please?
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?
Flags: needinfo?(aalldredge)
Blocking for 2.5. Non functional menu
blocking-b2g: 2.5? → 2.5+
Attached image 2.2 Long press menu
Attaching an image of the menu from Flame 2.2.
Flags: needinfo?(aalldredge)
IMO, The menu should remain as the same in 2.2. This is a regression.
Assignee: nobody → ferjmoreno
Attachment #8653499 - Flags: review?(borja.bugzilla)
Status: NEW → ASSIGNED
Attached image 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!
Flags: needinfo?(aalldredge)
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!
Attachment #8653499 - Flags: review?(borja.bugzilla)
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
Flags: needinfo?(aalldredge)
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.
QA Whiteboard: [QAnalyst-Triage+]
QA Contact: pcheng
I guess this was caused by bug 952456. Boris, could you give us some pointers, please? Thanks!
Assignee: ferjmoreno → nobody
Status: ASSIGNED → NEW
Component: Gaia::Contacts → Runtime
Flags: needinfo?(boris.chiou)
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.
Blocks: 952456
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Ni? is on Boris already.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
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.
Flags: needinfo?(boris.chiou)
Use a different array to store 'copy-image' and 'copy-link' menu items.
Attachment #8656432 - Flags: review?(kchen)
Attachment #8656434 - Flags: review?(timdream)
See Also: → 1201425
Attachment #8656432 - Flags: review?(kchen) → review+
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.
Flags: needinfo?(ferjmoreno)
Use a different array to store 'copy-image' and 'copy-link' menu items.

Fix mochitest.
Attachment #8656432 - Attachment is obsolete: true
Attachment #8656488 - Flags: review+
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.
Attachment #8656434 - Flags: review?(timdream) → review+
Use a different array to store 'copy-image' and 'copy-link' menu items.

Add more mochitests to test the content of menu objects.
Attachment #8656488 - Attachment is obsolete: true
Attachment #8656996 - Flags: review+
Flags: needinfo?(ferjmoreno)
Assignee: nobody → boris.chiou
Status: NEW → ASSIGNED
Keywords: checkin-needed
Hi, Sheriff
Could you please merge my gaia patch and gecko patch together? Thanks.
(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.
Keywords: checkin-needed
Keywords: checkin-needed
Summary: [Contacts] Long pressing on an image in edit mode will bring up a menu that does not function → Long pressing on an image in edit mode will bring up a menu that does not function
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
Flags: needinfo?(boris.chiou)
(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.
Flags: needinfo?(boris.chiou)
Attachment #8656434 - Attachment is obsolete: true
See Also: → 1203723
Blocks: 1203723
See Also: 1203723
See Also: → 1203726
Blocks: 1203726
See Also: 1203726
Depends on: 1204797
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)
Attachment #8656996 - Attachment is obsolete: true
Attachment #8660245 - Flags: review+
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.
Attachment #8662364 - Attachment is obsolete: true
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.
Attachment #8662919 - Flags: review?(kchen)
Attachment #8660245 - Flags: review?(timdream)
Blocks: 1206282
Attachment #8662919 - Flags: review?(kchen) → review+
Attachment #8660245 - Flags: review?(timdream) → review+
(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!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/574e834c0198
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S8 (02Oct)
Hi Tim,

Gecko part was merged. Could you please merge the gaia PR? Thanks.
Flags: needinfo?(timdream)
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... :)
Flags: needinfo?(timdream)
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
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
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?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: