Closed Bug 1205569 Opened 4 years ago Closed 3 years ago

[RTL] webconsole-filter-button icons have wrong side margin in RTL locales

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(firefox41 affected, firefox42 affected, firefox43 affected, firefox44 affected, firefox50 verified)

VERIFIED FIXED
Firefox 50
Tracking Status
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected
firefox44 --- affected
firefox50 --- verified

People

(Reporter: magicp.jp, Assigned: Kwan, Mentored)

References

(Blocks 2 open bugs)

Details

(Keywords: rtl)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20150916030203

Steps to reproduce:

1. Run any Firefox version in Arabic.
2. Open Web Console


Actual results:

webconsole-filter-button icons have wrong side margin in RTL locales.


Expected results:

webconsole-filter-button icons have correct side margin in RTL locales.
Blocks: 137995
Component: Untriaged → Developer Tools: Console
Keywords: rtl
OS: Unspecified → All
Hardware: Unspecified → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 43 Branch → unspecified
Whiteboard: [good first bug][lang=css]
Mentor: ntim.bugs
Do I need to know Arabic to resolve this bug? If not, I'd like to work on this bug. P.S. I've never contributed to this bug before.
(In reply to Tanay Prabhu Desai from comment #1)
> Do I need to know Arabic to resolve this bug? If not, I'd like to work on
> this bug. P.S. I've never contributed to this bug before.

Sorry, I meant I've never contributed to open source before.
(In reply to Tanay Prabhu Desai from comment #1)
> Do I need to know Arabic to resolve this bug?

No, you should be able to use the Force RTL addon to simulate a RTL locale: https://addons.mozilla.org/en-US/firefox/addon/force-rtl/
I installed that addon on both freshly built Nightly and my regular Firefox. It did not switch from LTR to RTL.
(In reply to Tanay Prabhu Desai from comment #4)
> I installed that addon on both freshly built Nightly and my regular Firefox.
> It did not switch from LTR to RTL.

Did you switch from menu? (menubar > Tools > Force RTL Direction)
Okay, I successfully reproduced the bug. I'd like to fix this bug.
Can this bug be assigned to me? I can try to fix it since its a good first bug?
(In reply to Tanay Prabhu Desai from comment #7)
> Can this bug be assigned to me? I can try to fix it since its a good first
> bug?

Sure!
Assignee: nobody → tanayseven
Status: NEW → ASSIGNED
Where to find the CSS file that I have to edit for the console? I'm searching in the directories in the central tree, could not find the file. Is it  this file mozilla-central/devtools/client/debugger/debugger.css?
(In reply to Tanay Prabhu Desai from comment #9)
> Where to find the CSS file that I have to edit for the console? I'm
> searching in the directories in the central tree, could not find the file.
> Is it  this file mozilla-central/devtools/client/debugger/debugger.css?

Here it is: https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/webconsole.css
I don't think I'll get enough time to fix this bug. I'm too busy with my college. You can assign the bug to someone else, if someone else is interested in fixing this bug.
(In reply to Tanay Prabhu Desai from comment #11)
> I don't think I'll get enough time to fix this bug. I'm too busy with my
> college. You can assign the bug to someone else, if someone else is
> interested in fixing this bug.

Alright, no problem. Thanks for the heads up!
Assignee: tanayseven → nobody
Status: ASSIGNED → NEW
I'd like to take this on, but before I submit a patch, I noticed simply adding the right pixel margins to the right of the icons fixed this without breaking ltr languages, that is without checking in css if the current direction is rtl (standard direction check didn't work, must be set up differently).

Is that simple margin addition an acceptable fix to submit if everything looks good?
Actually, these filter buttons might actually be going away soon-ish so I want to confirm before we spend any more time on it.  Helen, have you been working on new designs for the webconsole filters?  If so, are you thinking of changing the popup behavior we have now?
Flags: needinfo?(hholmes)
(In reply to Brian Grinstead [:bgrins] from comment #14)
> Actually, these filter buttons might actually be going away soon-ish so I
> want to confirm before we spend any more time on it.  Helen, have you been
> working on new designs for the webconsole filters?  If so, are you thinking
> of changing the popup behavior we have now?

I have been working on new designs for the webconsole filters. I think that it might be beneficial to hold off on this bug for now depending on the outcome of the meeting this afternoon on filtering.
Flags: needinfo?(hholmes)
Whiteboard: [good first bug][lang=css]
Priority: -- → P3
These are still around, and the fix is simple.
Assignee: nobody → moz-ian
Status: NEW → ASSIGNED
Attachment #8767488 - Flags: review?(ntim.bugs) → review+
Comment on attachment 8767488 [details]
Bug 1205569 - Use margin-inline-start instead of margin-left for the webconsole filter icons so they look right on RTL locales.

https://reviewboard.mozilla.org/r/61964/#review58700

Looks good to me! Thanks for picking this up.
Thanks for the fast review Tim!

Sheriff, I'm more than happy to do a try run, but given that this changes a CSS rule to one that usually resolves to the original rule anyway I think it'd be a waste of resources. Let me know if you disagree.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/54799c43ba43
Use margin-inline-start instead of margin-left for the webconsole filter icons so they look right on RTL locales. r=ntim
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/54799c43ba43
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
QA Whiteboard: [good first verify]
 hello, 


works well for me. web console bug is fixed. good team work guys
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.