1101816 - [RTL][Video] video controls are reversed in RTL context

Status: VERIFIED FIXED

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

Description

[Russ Nicoletti [:russn]]

Attachment: video controls in RTL context

The BiDi spec says "As per guidelines, playback controls are unchanged. Progress indicator is inversed". However, when the language is RTL, the video controls are reversed: the 'back' button appears on the right and the 'forward' button appears on the left.

Comment 1

[Russ Nicoletti [:russn]]

Stephany, I don't see anything in the video app css for this issue. Is it a gecko issue?

Comment 2

[Russ Nicoletti [:russn]]

Wilson, since the video controls are part of the gaia-header, I'm hoping you can provide some insight into this bug. Thanks.

Comment 3

[Stephany Wilkes]

Russ, I don't know the source of the issue. I know that Wilson had put some RTL support into gaia-header, as we'd hoped to have both web components and RTL support (that would include the back button change) in 2.2. This may be an artifact of that work, which changed when RTL support and web components were de-coupled for 2.2.

Comment 4

[Wilson Page [:wilsonpage]]

This is nothing to do with gaia-header. This is likely due to the video control being positioned using flexbox which reverses direction by default when `dir=rtl`.

Use this rule to override the default flexbox behaviour:

.controls {
  direction: ltr;
}

Comment 5

[Russ Nicoletti [:russn]]

Attachment: Github PR: https://github.com/mozilla-b2g/gaia/pull/26341

The fix for this bug is based on guidance from wilson

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8526194 [details] [review]
Github PR: https://github.com/mozilla-b2g/gaia/pull/26341

r+ but please add a comment explaining that this is intentionally ltr even in rtl locales and please wait for Wilson's feedback, on the issue below. I'm assuming that you've got a different bug filed for fixing the RTL bugs for the time slider and the duration and elapsed time displays, right?

Also, note that this could also be fixed with a dir attribute in the HTML instead of a direction property in the CSS. And it feels slightly more HTML-y to me than a CSS-y thing. Wilson: do you have an opinion or know what best practice is here? Fix it in CSS or in HTML?

Finally: note that Wilson is wrong about the controls being laid out in a flexbox. As far as I can tell, these elements are laid out with a normal flow, and so their direction is reversed. That is: this issue is not specific to flex boxes.

Comment 7

[Russ Nicoletti [:russn]]

Yes, there is a separate bug for the time slider moving ltr in rtl locales (bug 1095832). But there was no bug for the 'elapsed time' issue where the element is right aligned and thus produces too much space between the time and the slider; bug 1102611 has been create for that.

Comment 8

[Wilson Page [:wilsonpage]]

I'm not sure if the best practice is HTML `dir` attribute or CSS. In Camera app we're using CSS.

Comment 9

[David Flanagan [:djf]]

Thanks Wilson. I've convinced myself that this is a presentation issue and that CSS is the better way to deal with it, so using direction:ltr in this patch seems like the right way to go.

Comment 10

[Russ Nicoletti [:russn]]

Master: 

https://github.com/mozilla-b2g/gaia/commit/528d3d678f22e5cf5d5db8122c71a480fca13fb6

Comment 11

[Elie]

Attachment: Verify1_Flamev2.2&3.0_N5v2.2&3.0_Pass.png

This issue is verified pass on latest Flame v2.2&v3.0 and N5 v2.2&v3.0 build.
See attachment:Verify1_Flamev2.2&3.0_N5v2.2&3.0_Pass.png
Reproducing rate:0/10
STR:
1.Put some videos in DUT.
2.Launch Video and tap one video to play.
3.Pause the video.
Result:The view remain the same as that in LTR language except progress indicator is inversed.

Device: Flame 2.2 (pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.040918
Firmware Date          Thu May 21 04:09:27 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.193021
Firmware Date          Thu May 21 19:30:31 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.040628
Firmware Date          Thu May 21 04:06:43 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.193039
Firmware Date          Thu May 21 19:30:54 EDT 2015
Bootloader             HHZ12f

Comment 12

[Sue]

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