Closed Bug 1224124 Opened 6 years ago Closed 4 years ago

[Camera]The "Cancel" button on the action menu does not appear as an oval button.

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 unaffected, b2g-v2.5 affected, b2g-master affected)

RESOLVED WONTFIX
Tracking Status
b2g-v2.2 --- unaffected
b2g-v2.5 --- affected
b2g-master --- affected

People

(Reporter: wenqiuhong, Assigned: prateekjadhwani)

References

Details

(Keywords: regression)

Attachments

(4 files)

Attached file logcat_0208.txt
[1.Description]:
[Aries KK v2.5&2.6][Flame KK v2.5&2.6] The "Cancel" button on the action menu does not appear as an oval button after tapping the " ..."  icon on the preview page.

See attachments: Aries KK v2.6.3gp&Aries KK v2.6.png&logcat_0208.txt
Found Time: 02:08

[2.Testing Steps]: 
1. Launch the camera app.
2. Click the camera "shutter" button. 
3. Click the thumbnail of the photo just taken.
4. Tap the " ..." icon at the right top.

[3.Expected Result]: 
4. The "Cancel" button should appear as an oval button.

[4.Actual Result]: 
4. The Cancel button will appear as a rectangular button similar to other options in the menu, with its text left-aligned and there is a blank space below it at the bottom of the screen.

[5.Reproduction build]: 
Device: Aries KK v2.6 master( Affected )
Build ID               20151112014800
Gaia Revision          fdb66f75963fa9255f707af87f405d54892e5e7d
Gaia Date              2015-11-11 17:17:32
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ed7dd831d1969a5a1a8636e63bd93d6aeaf94a
Gecko Version          45.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151112.011957
Firmware Date          Thu Nov 12 01:20:05 UTC 2015
Bootloader             s1

Device: Aries KK v2.5( Affected )
Build ID               20151110094357
Gaia Revision          07baf613699fa6225359c7f04825c5caeb71d424
Gaia Date              2015-11-09 21:32:50
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e14287b00a514a15418dfaa89287030c588ad19d
Gecko Version          44.0a2
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151110.090331
Firmware Date          Tue Nov 10 09:03:39 UTC 2015
Bootloader             s1

Device: Flame KK v2.6 master 512mb( Affected )
Build ID               20151111150236
Gaia Revision          22f8023b112dfae83531b0a075ab9eb9a5444dfa
Gaia Date              2015-11-10 23:35:38
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/84a7cf29f4f14c9b359db2f7f19c0abd6a8e178e
Gecko Version          45.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151111.182640
Firmware Date          Wed Nov 11 18:26:52 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK v2.5 512mb( Affected )
Build ID               20151109004552
Gaia Revision          cf646c52bb947af28329b0a100df91d1b1f2a907
Gaia Date              2015-11-09 02:55:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4eafef5b80f8985c94c4a067f130d37513e1a581
Gecko Version          44.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151109.041411
Firmware Date          Mon Nov  9 04:14:26 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,10/10

[7.TCID]: 
Free Test
Note: 
This is a regression bug because it can't be repro on the Flame KK v2.2 512mb.
Device: Flame KK v2.2 512mb( Unaffected )
Build ID               20151111032502
Gaia Revision          885647d92208fb67574ced44004ab2f29d23cb45
Gaia Date              2015-10-07 13:05:24
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ac5fce5a78e5
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151111.064642
Firmware Date          Wed Nov 11 06:46:54 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0
Attached image Aries KK v2.6.png
Attached video Aries KK v2.6.3gp
Assignee: nobody → prateekjadhwani
Status: NEW → ASSIGNED
Hello Prateek,

Thank you very much for the patch. Unfortunately this is not quite right. Ideally we want to use the gaia-menu component which fixes this like in bug 1223684, or alternatively we can just adjust the markup to match what the building block CSS expects. Can you make the adjustment?
Thanks Kevin. Should I add you as a reviewr after the fix?
Flags: needinfo?(kevingrandon)
Comment on attachment 8686957 [details] [review]
[gaia] prateekjadhwani:1224124 > mozilla-b2g:master

Hi David, I was wondering if you could review the code and let me know if I need to make any changes.

Thanks
Attachment #8686957 - Flags: review?(dflanagan)
David or a camera peer would be a better reviewer than I. Your fix looks like the appropriate one at this time, though I did leave a comment on github. Thanks!
Flags: needinfo?(kevingrandon)
(In reply to Kevin Grandon :kgrandon from comment #8)
> David or a camera peer would be a better reviewer than I. Your fix looks
> like the appropriate one at this time, though I did leave a comment on
> github. Thanks!

Thanks Kevin for pointing me towards correct practices.
Comment on attachment 8686957 [details] [review]
[gaia] prateekjadhwani:1224124 > mozilla-b2g:master

Prateek,

Thanks for taking on this bug. Unfortunately, the Camera app is very different from the Video app in the way it manages and loads its dependencies, so the code from bug 1223684 won't really work in this case.

I'm not really active with the Camera app, and I don't know anything about about gaia-menu, so I'm not fully qualified to review this, but I'm pretty sure that those who are would have given r- because you're not using Bower and requirejs like the rest of the camera app does.

In comment #5, Kevin wrote "or alternatively we can just adjust the markup to match what the building block CSS expects".  I don't know what the underlying bug is, so I don't know what markup needs to be adjusted. I agree with Kevin that switching to gaia-menu is the ideal, but I suspect that given the complexities of the Camera app, the easier way to get this bug fixed and uplifted into 2.5, will be to continue using the existing CSS and modify the markup in apps/camera/js/views/preview-gallery.js.

If you don't know what those necessary changes might be, please set needinfo for Kevin.

In the future, Justin or Wilson will be better reviewers than I will be. I'm setting needinfo for them now so that they know about this bug and your work on it.
Flags: needinfo?(wilsonpage)
Flags: needinfo?(jdarcangelo)
Attachment #8686957 - Flags: review?(dflanagan) → review-
Please add any new shared/ dependencies into the requirejs config [1] and then `require()` them where needed.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/config/require.js
Flags: needinfo?(wilsonpage)
Prateek - Do you have time to address the review comments in comment 11? I would recommend switching to the gaia-menu component, up to camera peers about which one to use. (If we don't want UX changes, then using the one in shared/ is probably correct)
Flags: needinfo?(jdarcangelo) → needinfo?(prateekjadhwani)
(In reply to Kevin Grandon :kgrandon from comment #12)
> Prateek - Do you have time to address the review comments in comment 11? I
> would recommend switching to the gaia-menu component, up to camera peers
> about which one to use. (If we don't want UX changes, then using the one in
> shared/ is probably correct)

Yep, I will be making the changes as per the comments this weekend.
Thanks :)
Flags: needinfo?(prateekjadhwani)
Duplicate of this bug: 1230012
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.