Closed
Bug 1323504
Opened 8 years ago
Closed 8 years ago
Add RTL support for the Toolbox Options
Categories
(DevTools :: Framework, defect, P3)
DevTools
Framework
Tracking
(firefox53 fixed)
VERIFIED
FIXED
Firefox 53
| Tracking | Status | |
|---|---|---|
| firefox53 | --- | fixed |
People
(Reporter: itiel_yn8, Assigned: tomer)
References
(Blocks 1 open bug, )
Details
(Keywords: rtl)
Attachments
(2 files)
Non-RTL'd Toolbox Options.png
The Toolbox Options panel on RTL builds is currently not aligned to the right, which makes reading strings (containing both Hebrew and English words) unreadable.
Attached current look of the Toolbox Options on a Hebrew build.
| Comment hidden (mozreview-request) |
Component: Developer Tools → Developer Tools: Framework
Priority: -- → P3
|
Assignee | |
Comment 2•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8818611 [details]
Bug 1323504 - Add RTL support for the Toolbox Options
https://reviewboard.mozilla.org/r/98624/#review99606
::: devtools/client/framework/toolbox-options.xhtml:11
(Diff revision 1)
> <!ENTITY % toolboxDTD SYSTEM "chrome://devtools/locale/toolbox.dtd" >
> %toolboxDTD;
> +<!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
> + %globalDTD;
> ]>
> -<html xmlns="http://www.w3.org/1999/xhtml">
> +<html xmlns="http://www.w3.org/1999/xhtml" dir="&locale.dir;">
Should I replace it with dir="", as suggested on [bug 1323041 comment 6]?
[bug 1323041 comment 6]: https://bugzilla.mozilla.org/show_bug.cgi?id=1323041#c6?
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8818611 -
Flags: review?(jryans)
|
||
Comment 3•8 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8818611 [details]
Bug 1323504 - Add RTL support for the Toolbox Options
https://reviewboard.mozilla.org/r/98624/#review99606
> Should I replace it with dir="", as suggested on [bug 1323041 comment 6]?
>
> [bug 1323041 comment 6]: https://bugzilla.mozilla.org/show_bug.cgi?id=1323041#c6?
Yes, I think you should make this change.
I'll redirect your review to :jdescottes, who has worked with our RTL support more recently.
Attachment #8818611 -
Flags: review?(jryans) → review?(jdescottes)
|
||
Comment 4•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8818611 [details]
Bug 1323504 - Add RTL support for the Toolbox Options
https://reviewboard.mozilla.org/r/98624/#review100112
r+ with the global.dtd removed.
Note that the "camera" icon overlaps with the text of the "Screenshot Behvior".
Could be nice to add a #screenshot-icon:dir(rtl) selector to fix this, but not blocking the review on this.
Attachment #8818611 -
Flags: review?(jdescottes) → review+
|
|
Assignee | |
Comment 5•8 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8818611 [details]
Bug 1323504 - Add RTL support for the Toolbox Options
https://reviewboard.mozilla.org/r/98624/#review100112
The fix for the camera icon involves changing `left:50%` on `.devtools-button:empty::before, .devtools-toolbarbutton:not([label]):not([disabled]) > image` rule on `common.css`, so I'm considering it unsafe because it could affect other parts of devtools. I'll create another bug to fix it.
| Comment hidden (mozreview-request) |
|
|
||
Comment 7•8 years ago
|
||
Looks good.
We can't autoland your changeset until you mark the issue you opened on the review as "fixed" -> https://reviewboard.mozilla.org/r/98624/#comment128690
Can you update it? Thanks!
Flags: needinfo?(tomer.moz.bugs)
|
|
Assignee | |
Comment 8•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #7)
> We can't autoland your changeset
Sorry.
Flags: needinfo?(tomer.moz.bugs)
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c53a4c48e3f8
Add RTL support for the Toolbox Options r=jdescottes
|
||
Comment 10•8 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
|
||
Updated•8 years ago
|
QA Whiteboard: [good first verify]
|
Reporter | |
Comment 11•8 years ago
|
||
Yup, I can confirm this is fixed in latest Nightly.
|
||
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•