Closed Bug 1118713 Opened 10 years ago Closed 10 years ago

Browser chrome progress bar is not working as expected in RTL

Categories

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

All
Gonk (Firefox OS)
defect

Tracking

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

VERIFIED FIXED
2.2 S6 (20feb)
feature-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: nefzaoui, Assigned: mikehenrty)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files)

There is a bug in the browser chrome progress bar that makes it start from almost middle of the screen when in RTL.
Hey Kevin, Looking at the current UX spec for RTL* (page 15), it says we should be mirroring progress bars. Should I continue that part of the work here? Thanks! * https://mozilla.box.com/bidi-22
Flags: needinfo?(kgrandon)
For indeterminate progress bars, I personally don't think we should bother with it until we've done a UX study on it. For reference, we're currently using a backwards-moving implementation for this reason: http://www.chrisharrison.net/projects/progressbars2/ProgressBarsHarrison.pdf It doesn't seem like the study considers RTL, so if we don't have data on that, I think it would be good to do some research here/gather data. I don't think the implementation is worth the additional code complexity and upkeep cost that it introduces. It seems that for determinate progress bars we would definitely want to mirror the UI. Adding Stephany here for a comment.
Flags: needinfo?(kgrandon) → needinfo?(swilkes)
This is pretty clear in the RTL pattern, and we did gather data on it before documenting it in the pattern. What exactly needs clarification in the pattern? Let me know and thanks. I may be slow to respond as I’m on PTO in Mexico with slow-to-no Internet depending on the moment.
Or is this because the browser chrome is perhaps considered distinct from other progress bars?
RTL triage: P2 -- will make a best effort to get this into the 2.2 release.
Priority: -- → P2
RTL update: marking required bugs as feature-b2g:2.2+ (and removing blocking flags)
feature-b2g: --- → 2.2+
I think we can clear the NI here. We can do this, though it should probably be lower priority.
Flags: needinfo?(swilkes)
This is another bug with distinct issues being conflated. 1. As originally filed, a progress bar should NOT start from the middle of the progress bar, no matter which direction it flows. 2. Then, there is the "Which direction should the browser chrome animation flow in when a user has the Arabic language selected?" The bidi pattern does not actually specify a direction for the Browser Chrome: the pattern covers Progress and Activity indicators (both of which are what we call determinate progress bars) but not the Browser Chrome (which is an indeterminate progress bar). Today, the Browser Chrome animation (and animations for all indeterminate progress bars, if I'm not mistaken) flows right to left regardless of language selection. All a long way of saying: Arabic is already in good shape here, because the Browser Chrome animation already flows RTL by default, and thus matches the progress bar direction elsewhere as described in the bidi pattern. In sum: * No progress bar animation should begin in the middle of the bar. That's a bug. * The Browser Chrome progress bar in Arabic should flow RTL (as it normally does).
Whiteboard: [systemsfe]
Ahmed, are you working on this?
Flags: needinfo?(nefzaoui)
Comment on attachment 8560749 [details] [PullReq] mikehenrty:bug-1118713-progress-rtl to mozilla-b2g:master In RTL mode the progress element was aligning on the right side of the page, so animating to the left caused it to move off the page. Unfortunately we couldn't simply set text-align: left on the parent element since that doesn't work for block level children. Applying absolute position did the trick. Kevin, wanna take a look?
Flags: needinfo?(nefzaoui)
Attachment #8560749 - Flags: review?(kgrandon)
Assignee: nobody → mhenretty
Status: NEW → ASSIGNED
Comment on attachment 8560749 [details] [PullReq] mikehenrty:bug-1118713-progress-rtl to mozilla-b2g:master LGTM.
Attachment #8560749 - Flags: review?(kgrandon) → review+
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Keywords: checkin-needed
Doh, not a peer of that component, but this is in the system app, so landing: In master: https://github.com/mozilla-b2g/gaia/commit/0d7b35f23402c4cb29bca6b98280fec48a196dec
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Component: Gaia::Browser → Gaia::System::Browser Chrome
Resolution: --- → FIXED
Comment on attachment 8560749 [details] [PullReq] mikehenrty:bug-1118713-progress-rtl to mozilla-b2g:master Requesting uplift for this RTL fix. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): feature implementation. [User impact] if declined: Poor RTL experience in browser. [Testing completed]: Manual testing (tests are hard because it's css only). [Risk to taking this patch] (and alternatives if risky): Small commit, so should be low risk. [String changes made]: None.
Attachment #8560749 - Flags: approval-gaia-v2.2?(bbajaj)
Target Milestone: --- → 2.2 S5 (6feb)
Attachment #8560749 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Issue verified fixed on Flame 3.0 When language is set to Arabic, the progress bar in browser does not start from the middle of the screen, but is centered and element itself never moves from that position. The animation flows from right to left as expected, regardless of whether language being used is LTR or RTL. Device: Flame 3.0 Master Build ID: 20150209010211 Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec Gecko: 3436787a82d0 Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 38.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Adding verifyme for 2.2 verification.
Keywords: verifyme
Thi issue has bee verified successfully on Flame 2.2. Attachment:Verify_RTL_Browser.png Flame2.2 build: Build ID 20150214002504 Gaia Revision ea64caf6d4ab03fc4472eca9f41f20d651d55fa9 Gaia Date 2015-02-13 05:27:43 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6de30e6bbc84 Gecko Version 37.0a2 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150214.044811 Firmware Date Sat Feb 14 04:48:22 EST 2015 Bootloader L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+],[MGSEI-Triage+]
Keywords: verifyme
Attached image Verify_RTL_Browser.png (deleted) —
Test case has been added in moztrap: https://moztrap.mozilla.org/manage/case/15964/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: