1149919 - [RTL][Gallery]The spacing between text and right edge is more wider than that in LTR mode.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect, P3)

Description

[Sue]

Attachment: picture_details.png

[1.Description]:
[RTL][v2.2 & v3.0][Gallery]The spacing between text and right edge in RTL mode is more wider than spacing between text and left edge in LTR mode.
See attachment:picture_details.png

[2.Testing Steps]: 
Prerequisite: Have some pictures in device.
1. Set system language as Arabic.
2. Launch Gallery.
3. Select a picture to enter fullscreen view.
4. Tap the "i" button to check picture details.
5. Observe the spacing between text and right edge.

[3.Expected Result]: 
5. The spacing in RTL mode should be same as spacing between text and left edge in LTR mode.

[4.Actual Result]: 
5. The spacing in RTL mode is more wider than spacing between text and left edge in LTR mode.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150331002503
Gaia Revision          cc11248ab69f13e89416c8e6bb2e184187e72088
Gaia Date              2015-03-30 22:22:58
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/90a26917ee8f
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150331.034811
Firmware Date          Tue Mar 31 03:48:21 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (affected)
Build ID               20150331160205
Gaia Revision          03164bd160809747e6a198e0dba1b7c3ee7789f5
Gaia Date              2015-03-31 14:48:14
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/18a8ea7c2c62
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150331.191641
Firmware Date          Tue Mar 31 19:16:50 EDT 2015
Bootloader             L1TC000118D0

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

[7.TCID]: 
Free Test

Comment 1

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

Triage: P3, Polish

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1149919 > mozilla-b2g:master

Comment 3

[Punam Dahiya [:pdahiya]]

Comment on attachment 8586887 [details] [review]
[gaia] punamdahiya:Bug1149919 > mozilla-b2g:master

Hi David

Please review attached patch that fixes spacing of info screen from right edge in RTL mode. The attached patch fixes the issue reported in both gallery and Video app.

Thanks!

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8586887 [details] [review]
[gaia] punamdahiya:Bug1149919 > mozilla-b2g:master

I don't understand the second change in each file: why do you need to change the padding-left value for the dd element? Does that have any impact on the indentation of the text relative to the endpoints of the separator line? I assume it is there for a reason, though, and you can go ahead and land this patch as long as it does not modify the appearance of the anything in a LTR language.

Comment 5

[Punam Dahiya [:pdahiya]]

(In reply to David Flanagan [:djf] from comment #4)
> Comment on attachment 8586887 [details] [review]
> [gaia] punamdahiya:Bug1149919 > mozilla-b2g:master
> 
> I don't understand the second change in each file: why do you need to change
> the padding-left value for the dd element? Does that have any impact on the
> indentation of the text relative to the endpoints of the separator line? 

The second change in patch is changing padding-right from 0 to 1 rem for dd element so that it has 1rem space on right in RTL mode.


>I
> assume it is there for a reason, though, and you can go ahead and land this
> patch as long as it does not modify the appearance of the anything in a LTR
> language.

Comment 6

[Punam Dahiya [:pdahiya]]

Thanks David for review. I have tested and LTR info screen stays unchanged with patch.

Comment 7

[GitHub Autolander Bot]

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

Comment 8

[Punam Dahiya [:pdahiya]]

Comment on attachment 8586887 [details] [review]
[gaia] punamdahiya:Bug1149919 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Not a regression
[User impact] if declined: In RTL mode , the spacing of text from right edge will be less than spacing of text from left edge in LTR mode. The patch fixes this bug for both video and gallery app.
[Testing completed]: On master
[Risk to taking this patch] (and alternatives if risky):Very low
[String changes made]:None

Comment 9

[Sue]

Attachment: picture_details_v3.0_pass.png

This issue has been verified successfully on Flame 3.0 with the same steps in comment 0. The spacing in RTL mode ia same as spacing between text and left edge in LTR mode.
See attachment:picture_details_v3.0_pass.png
Rate:0/5

Device: Flame 3.0 (pass)
Build ID               20150402160202
Gaia Revision          62042ffcc8c6cca0f51ad23f5c2b979fc153b5a7
Gaia Date              2015-04-02 16:01:42
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/513265a4cbc2
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150402.192125
Firmware Date          Thu Apr  2 19:21:37 EDT 2015
Bootloader             L1TC000118D0

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Sue]

Attachment: v2.2_verify.png

This issue has been verified successfully on latest build of Flame 2.2 with the same steps in comment 0.
See attachment:v2.2_verify.png
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150406002503
Gaia Revision          a6351e1197d54f8624523c2db9ba1418f2aa046f
Gaia Date              2015-04-03 22:06:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c3335a5d3063
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150406.040047
Firmware Date          Mon Apr  6 04:00:58 EDT 2015
Bootloader             L1TC000118D0

Comment 12

[Sue]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15190/