1158740 - [Camera]The "Don‘t share" is truncated in Camera permission screen.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P2)

Description

[Elie]

[1.Description]:
[Flame][v2.2][Camera] Launch camera for the first time, then Camera permission screen pops up. You can see the "Don‘t share" is truncated.
Attchment:21-4.png

[2.Testing Steps]: 
1. Flash or reset the phone to bring up the FTE and finish the FTE to view the homescreen 
2. Tap 'Camera' from the home screen and wait for the Camera permission screen to pop up 


[3.Expected Result]: 
3.Nothing is not truncated in Camera permission screen.

[4.Actual Result]: 
3.The "Don't share" is truncated in  Camera permission screen.

[5.Reproduction build]: 
Device: Flame 2.1 user (unaffected)
Build ID               20150426001201
Gaia Revision          bbe983b4e8bebfec26b3726b79568a22d667223c
Gaia Date              2015-04-09 13:52:48
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/82a14be0462c
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150426.035220
Firmware Date          Sun Apr 26 03:52:31 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 2.2 user (affected)
Build ID               20150426002504
Gaia Revision          265ca0bc9408c21fc4b25a259fcee7fb642cd06b
Gaia Date              2015-04-24 19:13:28
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1908685d798d
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150426.040327
Firmware Date          Sun Apr 26 04:03:38 EDT 2015
Bootloader             L1TC000118D0

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

[7.TCID]: 
12760

Comment 1

[Elie]

Attachment: 21-4.png

Comment 2

[Delphine Lebédel [:delphine - Use needinfo]]

Same string size, so probably a regression between 2.1 and 2.2. 
Since this affects a shipping locale (and probably others), blocking 2.2 on this.

Comment 3

[David Flanagan [:djf]]

That is a system app permission dialog. It appears in the camera app (or any app that uses geolocation) but is not a camera bug, so changing the component.

Sam is working on bugs like this one and trying to figure out how and why the button margins changed.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Thanks David. It turns out bug 1010675 fixed some issues we had down in Layout with button content overflowing into padding. Our feeling (in bug 1157173) was that this was a good thing and that those instances should have never worked to begin with, so we're fixing them in Gaia, as we find them.

Comment 5

[David Flanagan [:djf]]

I looked around a little bit on this one....

Possibly the fix should go here: https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/system/style/themes/default/buttons.css#L17

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: permissions-buttons-padding.png

We need to revisit the default padding in https://github.com/mozilla-b2g/gaia/blob/master/shared/style/buttons.css#L20, which is currently set at 0 1.2rem. The fix in bug 1010675 changes the way these labels overflow, with the result that they will ellipsize sooner. This screenshot shows the result of setting left/right padding to 0.5rem - which is as big as I could get it without causing truncation/ellipsis. Obviously results may vary with different strings, fonts etc, but this example seems a reasonable proxy for instances where our existing string just barely fit inside the button. What do you think? I'll attach a patch as well so you can take it for a ride.

Comment 7

[GitHub Autolander Bot]

Attachment: [gaia] sfoster:shared-button-padding-bug-1158740 > mozilla-b2g:master

Comment 8

[Sam Foster [:sfoster] (he/him)]

Correction, we don't need to touch shared/style/buttons.css here, just the dialog buttons in apps/system/style/themes/default/buttons.css - so much less ocean to boil.

Comment 9

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8599493 [details] [review]
[gaia] sfoster:shared-button-padding-bug-1158740 > mozilla-b2g:master

Similar to bug 1157159, adjust dialog buttons styles to restore previous behavior after the overflow fix in bug 1010675

Comment 10

[Sam Foster [:sfoster] (he/him)]

Attachment: camera-permissions-dialog-pt-BR.png

With patch applied.

Comment 11

[Sam Foster [:sfoster] (he/him)]

Cancelling UX need-info, if we do this right there's no visual change. Also, see dev-gaia thread: https://groups.google.com/forum/#!topic/mozilla.dev.gaia/s3HyGQzk-Y4

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8599493 [details] [review]
[gaia] sfoster:shared-button-padding-bug-1158740 > mozilla-b2g:master

LGTM, thanks!

Comment 13

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/e358d81aa6e50ccf646040c0807f22d8abc80dec

Comment 14

[Josh Cheng [:josh]]

Hi Sam,
Could you help to raise approval request to land on 2.2? Thanks!

Comment 15

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8599493 [details] [review]
[gaia] sfoster:shared-button-padding-bug-1158740 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Permission dialog buttons, fix necessitated by Bug 1010675.
[User impact] if declined: Some button labels will be truncated in some languages, with their meaning perhaps unclear
[Testing completed]: Tested on device in multiple languages
[Risk to taking this patch] (and alternatives if risky): Low as patch is stylistic only, and affects these buttons specifically. Note that these CSS rules do apply to many dialogs system-wide however
[String changes made]: None

Comment 16

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

Attachment: fixed screenshot on 3.0

This issue is verified fixed on Flame 3.0. See screenshot for fixed behavior.

Device: Flame 3.0
BuildID: 20150506010204
Gaia: 3e6fd1e0a478af2c95d09ce95c2c6de2de2fec14
Gecko: ba44099cbd07
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Leaving verifyme tag for 2.2 verification after uplifting.

Comment 17

[Sam Foster [:sfoster] (he/him)]

v2.2: https://github.com/mozilla-b2g/gaia/commit/3d0e872798410d2f174fc63fba42dc22fc695767

Comment 18

[Hermes Cheng (inactive after July 27, 2015)]

Elie, could you help verify v2.2?

Comment 19

[Elie]

(In reply to Hermes Cheng[:hermescheng] from comment #18)
> Elie, could you help verify v2.2?

This issue verified successfully on flame 2.2
Device: Flame 2.2 user
Build ID               20150512002502
Gaia Revision          c4c1bf443f2b01c2ba918780510fd4c639a3c360
Gaia Date              2015-05-11 14:12:24
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/70782f19acbf
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150512.041710
Firmware Date          Tue May 12 04:17:22 EDT 2015
Bootloader             L1TC000118D0
Refer to picture    Don't share_verified pass.png

Comment 20

[Elie]

Attachment: Don't share_verified pass.png

Comment 21

[Shally Li（leave from Mozilla）]

Per Comment 16 & Comment 19, clear verifyme