Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[RTL] The Inspector rules view should be LTR

VERIFIED FIXED in Firefox 11

Status

()

Firefox
Developer Tools: Inspector
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: tomer, Assigned: paul)

Tracking

({rtl})

unspecified
Firefox 12
Points:
---

Firefox Tracking Flags

(firefox11- verified)

Details

(Whiteboard: [qa!])

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 575778 [details]
Screenshot

Given that code such as CSS and JavaScript is written in latin characters, I'd suggest making sure that the Style Inspector/Editor will be hardcoded to LTR even on RTL environments.

Steps to reproduce:
a. Launch RTL Firefox build, or use FORCE-RTL addon.
b. Go to any website, launch Inspector (Aurora→Developer Tools→Inspector).
c. Click Style button at the bottom left.

Expected result:
CSS code should not be right-aligned (direction:rtl), checkboxes should be at the left side of each line.
CCing Ehsan because I have a feeling that he's of the opinion that some locales do use RTL in CSS.

Maybe this bug should be to make reading direction separately configurable for Developer Tools.
OS: Linux → All
Hardware: x86 → All
(Reporter)

Comment 2

6 years ago
(In reply to Joe Walker from comment #1)
> Maybe this bug should be to make reading direction separately configurable
> for Developer Tools.
I'm not sure if it is needed, as I've never seen someone actually writing code in RTL environment, and I guess that if we'll have it configurable in the localization strings, all RTL locales will keep it as LTR.

Please also note that we keep other tools such as View Source, and the inspector HTML view as LTR, so the stylesheet should be kept as LTR as well because of consistently. 

The only exception I see here is if the rules are about to get translated (so instead of 'border' we should write it in other languages). If this is going to happen, it would make the situation more difficult and it might be better to leave it as RTL…
This bug seems to be about the style rules view of the Inspector - which I agree it needs more RTL-related polish. Tomer, please correct me if I am wrong.

The style editor we landed forces LTR for the editor.
Component: Developer Tools: Style Editor → Developer Tools: Inspector
QA Contact: developer.tools.style.editor → developer.tools.inspector
Summary: [RTL] Firefox Style Editor should be LTR → [RTL] The Inspector rules view should be LTR

Comment 4

6 years ago
Created attachment 584222 [details]
a more detailed screenshot showing what should and what should not be RTL

The actual rules must definitely be left-to-right in all locales, because CSS is inherently a left-to-right language.

The titles that say "inherited from X" should remain right-to-left in RTL locales, as they already are, because they have text that can be translated into a right-to-left language.
Attachment #584222 - Flags: ui-review+

Updated

6 years ago
Attachment #584222 - Flags: ui-review+
(Reporter)

Comment 5

6 years ago
This issue still exists in recent Aurora builds (and Beta?) and should be easy to resolve if we just force it to LTR as I suggested in comment 0. Please fix it before final release as we wish to see people actually using it.
(Assignee)

Comment 6

6 years ago
Created attachment 585386 [details]
RTL test

Is that better? (beside the treetwisties image being RTL)

Comment 7

6 years ago
Yes, this looks good.
(Assignee)

Comment 8

6 years ago
Created attachment 585390 [details] [diff] [review]
patch v1
(Assignee)

Updated

6 years ago
Attachment #585390 - Flags: review?(jwalker)
(Assignee)

Comment 9

6 years ago
Created attachment 585428 [details] [diff] [review]
patch v1.1
(Assignee)

Updated

6 years ago
Attachment #585390 - Attachment is obsolete: true
Attachment #585390 - Flags: review?(jwalker)
(Assignee)

Updated

6 years ago
Attachment #585428 - Flags: review?(jwalker)
Attachment #585390 - Flags: review+
Attachment #585428 - Flags: review?(jwalker) → review+
(Assignee)

Updated

6 years ago
Attachment #585428 - Flags: review?(dao)
Comment on attachment 585428 [details] [diff] [review]
patch v1.1

>--- a/browser/themes/gnomestripe/devtools/csshtmltree.css
>+++ b/browser/themes/gnomestripe/devtools/csshtmltree.css

>-.ruleview-expander:-moz-locale-dir(rtl) {
>-  background-position: 16px 0;
>-}

It looks like winstripe and pinstripe need this change as well.
Attachment #585428 - Flags: review?(dao) → review-
(Assignee)

Comment 11

6 years ago
Created attachment 586032 [details] [diff] [review]
patch v1.2

addressed Dao's comments
(Assignee)

Updated

6 years ago
Attachment #585428 - Attachment is obsolete: true
(Assignee)

Comment 12

6 years ago
(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 585428 [details] [diff] [review]
> patch v1.1
> 
> >--- a/browser/themes/gnomestripe/devtools/csshtmltree.css
> >+++ b/browser/themes/gnomestripe/devtools/csshtmltree.css
> 
> >-.ruleview-expander:-moz-locale-dir(rtl) {
> >-  background-position: 16px 0;
> >-}
> 
> It looks like winstripe and pinstripe need this change as well.

Hmm, you're right. I was convinced we were using this twisty only on Linux.
(Assignee)

Updated

6 years ago
Attachment #586032 - Flags: review?(dao)
Comment on attachment 586032 [details] [diff] [review]
patch v1.2

We really shouldn't be using this twisty anywhere, it's quite ugly.
Attachment #586032 - Flags: review?(dao) → review+
(Assignee)

Comment 14

6 years ago
Yeah. bug 699840
Whiteboard: [land-in-fx-team]
Do you know when/how this will be merged to mozilla-central?  Thanks!
Assignee: nobody → paul
Status: NEW → ASSIGNED
https://hg.mozilla.org/integration/fx-team/rev/2344d3d74214
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/2344d3d74214
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 12
(Reporter)

Comment 18

6 years ago
A user have submitted feedback requesting fixing this issue. 
http://input.mozilla.com/he/opinion/2574939
(Assignee)

Comment 19

6 years ago
(In reply to Tomer Cohen :tomer from comment #18)
> A user have submitted feedback requesting fixing this issue.
> http://input.mozilla.com/he/opinion/2574939

This issues is FIXED. If more work is needed for RTL support, can you please open a new bug with some more details about what is supposed to be done (like in attachment 584222 [details])?
(Reporter)

Comment 20

6 years ago
(In reply to Paul Rouget [:paul] from comment #19)
> This issues is FIXED. If more work is needed for RTL support, can you please
> open a new bug with some more details about what is supposed to be done
> (like in attachment 584222 [details])?

As far as I know, it is fixed only on Nightly, making it possible that we will release a broken feature. I can still reproduce it on recent Aurora builds.

Updated

6 years ago
tracking-firefox11: --- → ?

Comment 21

6 years ago
Comment on attachment 586032 [details] [diff] [review]
patch v1.2

[Approval Request Comment]
Regression caused by (bug #): New feature
User impact if declined: Rule View is confusing for people in RTL locales.
Testing completed (on m-c, etc.): Is on m-c for 2 weeks, aurora since the merge.
Risk to taking this patch (and alternatives if risky): CSS patch, not much risk.
String changes made by this patch: None.
Attachment #586032 - Flags: approval-mozilla-beta?
Comment on attachment 586032 [details] [diff] [review]
patch v1.2

[Triage Comment]
Issue with new feature and low risk - approved for Beta 11.
Attachment #586032 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/c9e908215a05
status-firefox11: --- → fixed

Updated

6 years ago
tracking-firefox11: ? → -
Whiteboard: [qa+]

Comment 24

6 years ago
I have followed the steps from the description on FA locale:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0 beta 3
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0 beta 3
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0 beta 3

The checkboxes areat the left side of each line.
Setting the resolution to Verified Fixed on beta.
Status: RESOLVED → VERIFIED
status-firefox11: fixed → verified
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.