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)
Firefox OS Graveyard
Gaia::System::Browser Chrome
All
Gonk (Firefox OS)
Tracking
(feature-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: nefzaoui, Assigned: mikehenrty)
References
Details
(Whiteboard: [systemsfe])
Attachments
(2 files)
(deleted),
text/x-github-pull-request
|
kgrandon
:
review+
bajaj
:
approval-gaia-v2.2+
|
Details |
(deleted),
image/png
|
Details |
There is a bug in the browser chrome progress bar that makes it start from almost middle of the screen when in RTL.
Reporter | ||
Updated•10 years ago
|
Blocks: browser-window-rtl
Reporter | ||
Comment 1•10 years ago
|
||
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)
Comment 2•10 years ago
|
||
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)
Comment 3•10 years ago
|
||
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.
Comment 4•10 years ago
|
||
Or is this because the browser chrome is perhaps considered distinct from other progress bars?
Comment 5•10 years ago
|
||
RTL triage: P2 -- will make a best effort to get this into the 2.2 release.
Priority: -- → P2
Comment 6•10 years ago
|
||
RTL update: marking required bugs as feature-b2g:2.2+ (and removing blocking flags)
feature-b2g: --- → 2.2+
Comment 7•10 years ago
|
||
I think we can clear the NI here. We can do this, though it should probably be lower priority.
Flags: needinfo?(swilkes)
Comment 8•10 years ago
|
||
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).
Assignee | ||
Updated•10 years ago
|
Whiteboard: [systemsfe]
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → mhenretty
Status: NEW → ASSIGNED
Comment 12•10 years ago
|
||
Comment on attachment 8560749 [details]
[PullReq] mikehenrty:bug-1118713-progress-rtl to mozilla-b2g:master
LGTM.
Attachment #8560749 -
Flags: review?(kgrandon) → review+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 13•10 years ago
|
||
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.
Updated•10 years ago
|
Keywords: checkin-needed
Comment hidden (obsolete) |
Comment 15•10 years ago
|
||
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 16•10 years ago
|
||
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)
Updated•10 years ago
|
Target Milestone: --- → 2.2 S5 (6feb)
Updated•10 years ago
|
Attachment #8560749 -
Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Comment 17•10 years ago
|
||
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?]
status-b2g-master:
--- → verified
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 19•10 years ago
|
||
status-b2g-v2.2:
--- → fixed
Target Milestone: 2.2 S5 (6feb) → 2.2 S6 (20feb)
Comment 20•10 years ago
|
||
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
Comment 21•10 years ago
|
||
Comment 22•10 years ago
|
||
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.
Description
•