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

VERIFIED FIXED in 2.2 S1 (5dec)

Status

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: rnicoletti, Assigned: rnicoletti)

Tracking

unspecified
2.2 S1 (5dec)
x86
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(feature-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: video-rtl [ft:media])

Attachments

(3 attachments)

Assignee

Description

5 years ago
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.
Assignee

Updated

5 years ago
Blocks: video-rtl
Assignee

Comment 1

5 years ago
Stephany, I don't see anything in the video app css for this issue. Is it a gecko issue?
Flags: needinfo?(swilkes)
Whiteboard: video-rtl
Assignee

Comment 2

5 years ago
Wilson, since the video controls are part of the gaia-header, I'm hoping you can provide some insight into this bug. Thanks.
Flags: needinfo?(swilkes) → needinfo?(wilsonpage)

Comment 3

5 years ago
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.
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;
}
Flags: needinfo?(wilsonpage)
Assignee

Comment 5

5 years ago
The fix for this bug is based on guidance from wilson
Attachment #8526194 - Flags: review?(dflanagan)

Updated

5 years ago
Assignee: nobody → rnicoletti
feature-b2g: --- → 2.2+
Whiteboard: video-rtl → video-rtl [ft:media]
Target Milestone: --- → 2.2 S1 (5dec)
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.
Flags: needinfo?(wilsonpage)
Attachment #8526194 - Flags: review?(dflanagan) → review+
Assignee

Comment 7

5 years ago
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.
I'm not sure if the best practice is HTML `dir` attribute or CSS. In Camera app we're using CSS.
Flags: needinfo?(wilsonpage)
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.
Assignee

Comment 10

5 years ago
Master: 

https://github.com/mozilla-b2g/gaia/commit/528d3d678f22e5cf5d5db8122c71a480fca13fb6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Comment 11

4 years ago
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

Updated

4 years ago
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]

Comment 12

4 years ago
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15347/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.