Closed
Bug 1217784
Opened 9 years ago
Closed 9 years ago
[RTL][Browser]The URL is left-aligned.
Categories
(Firefox OS Graveyard :: Gaia::Browser, defect, P2)
Tracking
(b2g-v2.5 verified, b2g-master verified)
VERIFIED
FIXED
FxOS-S11 (13Nov)
People
(Reporter: yulan.zhu, Assigned: autra)
References
Details
(Whiteboard: [2.5-rtl-test-run])
Attachments
(4 files)
[1.Description]: [RTL][AriesKK&FlameKK v2.5][Browser]Go to a website, the URL is left-aligned. See attachment:Browser_URL_v2.5.png [2.Testing Steps]: 1. Set system language as Arabic. 2. Click on the Browser app. 3. Tap the address bar. 4. Input website address and tap Search button on keyboard. >> 4.1. The "..." (Menu) icon should be left-aligned. 4.2. The URL should be right-aligned but read left to right. [3.Expected Result]: 4.The URL should be right-aligned and read left to right. [4.Actual Result]: 4.The URL is left-aligned. [5.Reproduction build]: Device: Flame KK 2.5 build (Affected) Build ID 20151022150207 Gaia Revision 29ce8ec8606e59f582375234440812b046346513 Gaia Date 2015-10-22 05:31:38 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/76bd0c01d72e64ca4f261ffdb2652a91f961e930 Gecko Version 44.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20151022.185000 Firmware Date Thu Oct 22 18:50:13 EDT 2015 Firmware Version v18D v4 Bootloader L1TC000118D0 Device: Aries KK 2.5 build (Affected) Build ID 20151023005002 Gaia Revision 29ce8ec8606e59f582375234440812b046346513 Gaia Date 2015-10-22 05:31:38 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/1f03a14106e59280761ac53904340f389674337f Gecko Version 44.0a1 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20151023.001128 Firmware Date Fri Oct 23 00:11:35 UTC 2015 Bootloader s1 [6.Reproduction Frequency]: Always Recurrence,10/10 [7.TCID]: 15466
Reporter | ||
Updated•9 years ago
|
Blocks: 1181921
QA Whiteboard: [2.5-rtl-test-run]
status-b2g-master:
--- → affected
Whiteboard: [2.5-rtl-test-run]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → augustin.trancart
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master Hi Ben, Please r?
Attachment #8679965 -
Flags: review?(bfrancis)
Updated•9 years ago
|
Priority: -- → P2
Comment 3•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master Thanks for the patch, This patch seems very drastic for the issue described in this bug, it would have effect on the whole system app. If you're just trying to fix the problem described in this bug I would suggest a solution more isolated to the browser chrome. If you are trying to change the default for the whole system app I would suggest maybe this isn't the right bug or the right CSS file to do that in. What are you trying to achieve?
Attachment #8679965 -
Flags: review?(bfrancis) → review-
Assignee | ||
Comment 4•9 years ago
|
||
Hi Ben, thanks for the review! Yes, it looks a bit drastic. However, I don't think we are taking a big risk there: 1/ for elements that have neither direction nor text-align specified, this is the default anyway (the default is 'start', but in this case, that's the same). 2/ for elements that have a dir="auto" (added for RTL most probably), this is what we want in most of the case : the calculated direction of the content of an element should not change the alignment. We are restoring what happens without the dir="auto". I'm saying "most of", but the only case where this is not true is for multiline contenteditable elements. 3/ for elements (either with or without dir) that have a text-align specified, their rule will always override the inherited text-align. We used this to cleanly fix alignment problems (what I describe in 2/ actually) in other apps, with success. That being said, chrome.css is clearly not the right place to put this, indeed. Do you have a suggestion for another place where I can put that? Or I can restrict that to .chrome or other class if you still think this change is too risky of course. Thanks!
Flags: needinfo?(bfrancis)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Ben Francis [:benfrancis] from comment #3) > Comment on attachment 8679965 [details] [review] > [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master > What are you trying to achieve? I didn't answer this: I'm trying to fix all alignment issues once and for all :-) I might be too ambitious though...
Comment 6•9 years ago
|
||
Sam or Tim, do you have a suggestion for a better place to put this global CSS rule?
Flags: needinfo?(timdream)
Flags: needinfo?(sfoster)
Flags: needinfo?(bfrancis)
Comment 7•9 years ago
|
||
Thinking about this. I get how the rule works and it maybe a succinct way to solve a lot of problems. But I really think a patch like this should be attached to a bug that describes the problem it is solving. On a good day this would make me a bit fidgety. On the day before RA I think its reckless. Lets make a chrome-specific patch and file a follow-up for any further alignment issues.
Flags: needinfo?(sfoster)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master Hey Ben, Here is a more reasonable patch :-) Please r? again.
Attachment #8679965 -
Flags: review- → review?(bfrancis)
Comment 9•9 years ago
|
||
What you asked in comment 6 is probably rebased and gone in comment 8. The current patch in the pull request (changes the .urlbar only) looks to me like a right fix (and in the right place). As a side note, this is probably a really really slow CSS selector -- I wonder if there has ever been a discussion on this somewhere. If not we need to talk to the platform team and ask if there is a better way to specify rtl style.
Flags: needinfo?(timdream)
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #9) > > As a side note, this is probably a really really slow CSS selector -- I > wonder if there has ever been a discussion on this somewhere. If not we need > to talk to the platform team and ask if there is a better way to specify rtl > style. Are you talking about html[dir="ltr"] alone? Doesn't the engine try to find all html elements first ? That would be very fast. But maybe it tries to match all [dir="ltr"]... I'd like to hear from platform engineer indeed :-) That being said, I'm not sure we really have an alternative: usually we really want to test the document direction, not the element's inherited direction (they are different is some cases). For this particular case, we match 3 classes before. Even though the child selector is slow, I don't expect many elements will be matched after the first class .urlbar anyway, so walking up the tree should not be a real problem, WDYT?
Comment 11•9 years ago
|
||
> For this particular case, we match 3 classes before. Even though the child
> selector is slow, I don't expect many elements will be matched after the
> first class .urlbar anyway, so walking up the tree should not be a real
> problem, WDYT?
Its a pattern we use throughout Gaia, I think Tim was just saying we should investigate (post 2.5). We're just waiting for Ben's review here to land.
Comment 12•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master Thank you!
Attachment #8679965 -
Flags: review?(bfrancis) → review+
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 13•9 years ago
|
||
NI myself to land this when tests are green
Flags: needinfo?(augustin.trancart)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(augustin.trancart)
Keywords: checkin-needed
Comment 14•9 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/3dff75eb3c4bd59fe1862eae7d893783381218a4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S11 (13Nov)
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #):None [User impact] if declined: Bad RTL support of the browser app [Testing completed]: on Flame master [Risk to taking this patch] (and alternatives if risky): Very low risk, only a text-alignment in CSS [String changes made]: None
Attachment #8679965 -
Flags: approval-gaia-v2.5?
Reporter | ||
Comment 16•9 years ago
|
||
This issue is verified as pass on latest FlameKK&AriesKK master build by the same STR in comment 0. Actual result:The URL is right-aligned and read left to right. See attachment:Verify1_URL_master.png. Reproducing rate:0/10 Device:FlameKK master build(512mb) (Pass) Build ID 20151103150203 Gaia Revision 61918ddd9ccce104c009e873e34a0791e125753a Gaia Date 2015-11-03 17:22:30 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/f742b9412ed5aace90ad863b276faae0641090a8 Gecko Version 45.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20151103.182550 Firmware Date Tue Nov 3 18:26:03 EST 2015 Firmware Version v18D v4 Bootloader L1TC000118D0 Device: AriesKK master build (Pass) Build ID 20151104004249 Gaia Revision 61918ddd9ccce104c009e873e34a0791e125753a Gaia Date 2015-11-03 17:22:30 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/f742b9412ed5aace90ad863b276faae0641090a8 Gecko Version 45.0a1 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20151104.000116 Firmware Date Wed Nov 4 00:01:24 UTC 2015 Bootloader s1
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [2.5-rtl-test-run] → [rtl-impact][MGSEI-Triage+]
status-b2g-v2.5:
--- → affected
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8679965 [details] [review] [gaia] Phoxygen:bug1217784-browser_url_alignment > mozilla-b2g:master Already in 2.5
Attachment #8679965 -
Flags: approval-gaia-v2.5?
Comment 18•9 years ago
|
||
This bug has been verified as "pass" on the latest build of Flame KK v2.5 512mb and Aries KK v2.5 by the STR in comment 0 . Actual result: The URL is right-aligned and read left to right. See attachment: verified_Aries_KK_v2.5_URL.png Reproduce rate: 0/10. Device: Aries KK v2.5(Pass) Build ID 20151110094357 Gaia Revision 07baf613699fa6225359c7f04825c5caeb71d424 Gaia Date 2015-11-09 21:32:50 Gecko Revision http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e14287b00a514a15418dfaa89287030c588ad19d Gecko Version 44.0a2 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20151110.090331 Firmware Date Tue Nov 10 09:03:39 UTC 2015 Bootloader s1 Device: Flame KK v2.5 512mb(Pass) Build ID 20151109004552 Gaia Revision cf646c52bb947af28329b0a100df91d1b1f2a907 Gaia Date 2015-11-09 02:55:50 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4eafef5b80f8985c94c4a067f130d37513e1a581 Gecko Version 44.0a2 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20151109.041411 Firmware Date Mon Nov 9 04:14:26 EST 2015 Firmware Version v18D v4 Bootloader L1TC000118D0
Status: RESOLVED → VERIFIED
Comment 19•9 years ago
|
||
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•