[RTL] Fix general twisty direction issue in RTL locale

RESOLVED FIXED in Firefox 51

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: magicp, Assigned: steveck)

Tracking

(Blocks: 2 bugs, {rtl})

unspecified
Firefox 51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox42 affected, firefox43 affected, firefox44 affected, firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8662269 [details]
rtl-devtools-performance-waterfall-twisty.png

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 version Firefox in Arabic.
2. Open "http://helloracer.com/webgl/"
3. Open Performance in Developer Tools.
4. Run recording.
5. Stop recording.
6. Select [Waterfall]


Actual results:

Twisty direction is wrong in RTL locales


Expected results:

Twisty direction is correct in RTL locales
(Reporter)

Updated

3 years ago
Blocks: 137995
Component: Untriaged → Developer Tools: Performance Tools (Profiler/Timeline)
Keywords: rtl
OS: Unspecified → All
Hardware: Unspecified → All
(Reporter)

Updated

3 years ago
Status: UNCONFIRMED → NEW
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
status-firefox44: --- → affected
Ever confirmed: true
Version: 43 Branch → unspecified
Triaging. Filter on ADRENOCORTICOTROPIC (yes).
Priority: -- → P3
(Assignee)

Comment 2

2 years ago
Created attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

Review commit: https://reviewboard.mozilla.org/r/69498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69498/
Attachment #8778115 - Flags: review?(ntim.bugs)
(Assignee)

Comment 3

2 years ago
https://reviewboard.mozilla.org/r/69498/#review66640

::: devtools/client/themes/dark-theme.css:287
(Diff revision 1)
>    visibility: hidden;
>  }
>  
> +/* Mirror the twisty for rtl direction */
> +.theme-twisty:dir(rtl),
> +.theme-twisty:-moz-locale-dir(rtl) {

Maybe we should even remove the transform in .expander and simply leverage the .theme-twisty to handle all the twisties, wdyt?
(Assignee)

Updated

2 years ago
Assignee: nobody → schung
Status: NEW → ASSIGNED

Comment 4

2 years ago
(In reply to Steve Chung [:steveck] from comment #3)
> https://reviewboard.mozilla.org/r/69498/#review66640
> 
> ::: devtools/client/themes/dark-theme.css:287
> (Diff revision 1)
> >    visibility: hidden;
> >  }
> >  
> > +/* Mirror the twisty for rtl direction */
> > +.theme-twisty:dir(rtl),
> > +.theme-twisty:-moz-locale-dir(rtl) {
> 
> Maybe we should even remove the transform in .expander and simply leverage
> the .theme-twisty to handle all the twisties, wdyt?

Sounds good, it makes things consistent and avoids code duplication.

Comment 5

2 years ago
mozreview-review
Comment on attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

https://reviewboard.mozilla.org/r/69498/#review66742
Attachment #8778115 - Flags: review?(ntim.bugs)

Comment 6

2 years ago
(In reply to Steve Chung [:steveck] from comment #3)
> https://reviewboard.mozilla.org/r/69498/#review66640
> 
> ::: devtools/client/themes/dark-theme.css:287
> (Diff revision 1)
> >    visibility: hidden;
> >  }
> >  
> > +/* Mirror the twisty for rtl direction */
> > +.theme-twisty:dir(rtl),
> > +.theme-twisty:-moz-locale-dir(rtl) {
> 
> Maybe we should even remove the transform in .expander and simply leverage
> the .theme-twisty to handle all the twisties, wdyt?

Yes, I like the idea.
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

https://reviewboard.mozilla.org/r/69498/#review67588

The console twisties are in the wrong direction. r=me with the console issue fixed (an override setting transform: none in webconsole.css should suffice)
Attachment #8778115 - Flags: review?(ntim.bugs) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 10

2 years ago
Patch updated, thanks!
Keywords: checkin-needed

Comment 11

2 years ago
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0c68d5900cce
[RTL] Waterfall twisty direction is wrong in RTL locales. r=ntim
Keywords: checkin-needed

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c68d5900cce
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
(Reporter)

Comment 13

2 years ago
(In reply to Steve Chung [:steveck] from comment #10)
> Patch updated, thanks!

Hi Steve, Is this patch affect direction of .ruleview-expander.theme-twisty ?
Flags: needinfo?(schung)
(Reporter)

Comment 14

2 years ago
Hi Steve, This patch does not effect waterfall twisty direction in latest Nightly. This because .arrow.twisty class has transform style in inline.

However, fixing waterfall twisty direction may no longer need. Because waterfall-view should be drawn from left to right (same as graph-canvas) I think. If we need to change waterfall-view direction from "rtl" to "ltr", waterfall twisty is in as-is. What do you think about this?
(Assignee)

Comment 15

2 years ago
Hi magicp, I think you're right that this patch didn't fix the twisty in waterfall... I remember that I noticed there is another related bug 1205584, and this patch was for fixing bug 1205584 instead waterfall twisty. Sorry about the confusing... :/ I can switch the title and attachment in both bugs and follow up the waterfall twisty issue in bug 1205584 since it not easy to be fixed.

BTW I don't think the the waterfall chart is drawn as canvas. It's still the xul and I'm not sure if we plan to change it to HTML canvas. The layout in waterfall seems busted because the position was generated at runtime in js. Maybe we should force the waterfall to ltr for now and propose a better waterfall with HTML.
Flags: needinfo?(schung)
(Reporter)

Comment 16

2 years ago
I'd like to close this bug. Then I will raise new bug reports about .ruleview-expander.theme-twisty and waterfall-view. Could you please change to appropriate title for this? Thanks.
(Assignee)

Updated

2 years ago
Summary: [RTL] Waterfall twisty direction is wrong in RTL locales → [RTL] Fix general twisty direction issue in RTL locale
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1205584
(Assignee)

Comment 18

2 years ago
(In reply to magicp from comment #16)
> I'd like to close this bug. Then I will raise new bug reports about
> .ruleview-expander.theme-twisty and waterfall-view. Could you please change
> to appropriate title for this? Thanks.

Thanks for filing a new bug, I also close Bug 1205584 because of this patch.

Updated

2 years ago
Depends on: 1296648
You need to log in before you can comment on or make changes to this bug.