1205590 - [RTL] Fix general twisty direction issue in RTL locale

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)

Description

[magicp]

Attachment: 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

Comment 1

[Victor Porof [:vporof][:vp]]

Triaging. Filter on ADRENOCORTICOTROPIC (yes).

Comment 2

[Steve Chung [:steveck]]

Attachment: 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/

Comment 3

[Steve Chung [:steveck]]

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?

Comment 4

[Tim Nguyen :ntim]

(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

[Tim Nguyen :ntim]

Comment on attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

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

Comment 6

[Tim Nguyen :ntim]

(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 7

[Steve Chung [:steveck]]

Comment on attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69498/diff/1-2/

Comment 8

[Tim Nguyen :ntim]

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)

Comment 9

[Steve Chung [:steveck]]

Comment on attachment 8778115 [details]
Bug 1205590 - [RTL] Waterfall twisty direction is wrong in RTL locales.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69498/diff/2-3/

Comment 10

[Steve Chung [:steveck]]

Patch updated, thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c68d5900cce
[RTL] Waterfall twisty direction is wrong in RTL locales. r=ntim

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0c68d5900cce

Comment 13

[magicp]

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

Hi Steve, Is this patch affect direction of .ruleview-expander.theme-twisty ?

Comment 14

[magicp]

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?

Comment 15

[Steve Chung [:steveck]]

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.

Comment 16

[magicp]

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.

Comment 18

[Steve Chung [:steveck]]

(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.