Ada [:adalucinet] Reporter
	Description • 8 years ago

[Affected versions]:
*reproducible only with RTL builds
- Firefox 47.0.1 (Build ID: 20160623154057)
- Firefox 48 beta 6 (Build ID: 20160706215822)
- latest Aurora 49.0a1
- latest Nightly 50.0a1

[Affected platforms]:
- Windows 10 64-bit
- Mac OS X 10.9.5
- Ubuntu 14.04 x86

[Steps to reproduce]:
1. Launch l10n Firefox build.
2. Open Inspector: Ctrl + Shift + C (for Windows & Ubuntu) or Cmd + Opt + C (for Mac OS X)

[Expected result]: Collapse/Expand pane button is properly displayed.

[Actual result]: Collapse/Expand pane button is not properly displayed.

[Regression range]: 
- Also reproducible with the oldest available l10n Nightly build on the ftp - 45.0a1 (from 2015-11-02); I guess this is a long standing issue.

[Additional notes]:
- Screenshot with both normal and RTL builds → https://i.imgur.com/46FrV18.png

	Marco Mucci [:MarcoM]
	Comment 1 • 8 years ago

Hi Alexandra, which bug is this related to?

	Marco Mucci [:MarcoM]
	Comment 2 • 8 years ago

Is there a specific Bug ID I can make this bug block?

	Marco Mucci [:MarcoM]
	Comment 3 • 8 years ago

(In reply to Marco Mucci [:MarcoM] from comment #2)
> Is there a specific Bug ID I can make this bug block?

	Ada [:adalucinet] Reporter
	Comment 4 • 8 years ago

(In reply to Marco Mucci [:MarcoM] from comment #3)
> (In reply to Marco Mucci [:MarcoM] from comment #2)
> > Is there a specific Bug ID I can make this bug block?

Hey Marco! This issue was found while verifying bug 1266420; but, as far as I can see, it's a long standing bug, definitely not a recent regression.

	Marco Mucci [:MarcoM]
	Comment 5 • 8 years ago

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #4)
> (In reply to Marco Mucci [:MarcoM] from comment #3)
> > (In reply to Marco Mucci [:MarcoM] from comment #2)
> > > Is there a specific Bug ID I can make this bug block?
> 
> Hey Marco! This issue was found while verifying bug 1266420; but, as far as
> I can see, it's a long standing bug, definitely not a recent regression.

Thanks very much Alexandra.  Track #2 team will triage this bug.

	Marco Mucci [:MarcoM]
	Comment 6 • 8 years ago

Waiting on team to triage tomorrow.

	gasolin@mozilla.com Assignee
	Comment 7 • 8 years ago

seems pseudo-class(::-moz-dir) with pseudo-element(:before) does not work

> - .sidebar-toggle::before {
> + .sidebar-toggle::before,
> + .sidebar-toggle.pane-collapsed:-moz-dir(rtl)::before {
>   background-image: var(--theme-pane-collapse-image);
> }

Does it a bug of pseudo selector, or should I assign image in code instead of in CSS?

	Tim Nguyen :ntim
	Comment 8 • 8 years ago

(In reply to Fred Lin [:gasolin] from comment #7)
> seems pseudo-class(::-moz-dir) with pseudo-element(:before) does not work
> 
> > - .sidebar-toggle::before {
> > + .sidebar-toggle::before,
> > + .sidebar-toggle.pane-collapsed:-moz-dir(rtl)::before {
> >   background-image: var(--theme-pane-collapse-image);
> > }
> 
> Does it a bug of pseudo selector, or should I assign image in code instead
> of in CSS?
Your approach should work, it's just a matter of choosing the right selector :)

::-moz-dir has been replaced by ::dir. ::dir only works on HTML documents, while ::-moz-locale-dir works for XUL documents.

	gasolin@mozilla.com Assignee
	Comment 9 • 8 years ago

-moz-locale-dir works, thanks! I was thinking the div button is a normal html element :/

	gasolin@mozilla.com Assignee
	Comment 10 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/67096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67096/

	Tim Nguyen :ntim
	Comment 11 • 8 years ago

(In reply to Fred Lin [:gasolin] from comment #9)
> -moz-locale-dir works, thanks! I was thinking the div button is a normal
> html element :/

It is, except inspector.xul is a XUL doc.

	Tim Nguyen :ntim
	Comment 12 • 8 years ago

Comment on attachment 8774638 [details]
Bug 1285528 - [RTL] Collapse/Expand pane button is not properly displayed;

https://reviewboard.mozilla.org/r/67096/#review63948

::: devtools/client/shared/components/sidebar-toggle.css:10
(Diff revision 1)
>  
>  .sidebar-toggle {
>    display: block;
>  }
>  
> -.sidebar-toggle::before {
> +.sidebar-toggle::before,

Can you change this to .sidebar-toggle:-moz-locale-dir(ltr) ?

::: devtools/client/shared/components/sidebar-toggle.css:15
(Diff revision 1)
> -.sidebar-toggle::before {
> +.sidebar-toggle::before,
> +.sidebar-toggle.pane-collapsed:-moz-locale-dir(rtl)::before {
>    background-image: var(--theme-pane-collapse-image);
>  }
>  
> -.sidebar-toggle.pane-collapsed::before {
> +.sidebar-toggle.pane-collapsed::before,

Same thing here

::: devtools/client/shared/components/sidebar-toggle.css:27
(Diff revision 1)
> +  .sidebar-toggle:-moz-locale-dir(rtl)::before {
> +    transform: rotate(-90deg);
> +  }

Does the sidebar have a different location (top instead of bottom) on RTL for widths < 700px ? If not, can you remove this ?

	gasolin@mozilla.com Assignee
	Comment 13 • 8 years ago

https://reviewboard.mozilla.org/r/67096/#review63948

> Can you change this to .sidebar-toggle:-moz-locale-dir(ltr) ?

done

> Does the sidebar have a different location (top instead of bottom) on RTL for widths < 700px ? If not, can you remove this ?

In RTL mode the icon is changed to the conter-part, so the icon has to rotate in counter direction as well.

	gasolin@mozilla.com Assignee
	Comment 14 • 8 years ago

Comment on attachment 8774638 [details]
Bug 1285528 - [RTL] Collapse/Expand pane button is not properly displayed;

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

	Tim Nguyen :ntim
	Comment 15 • 8 years ago

Comment on attachment 8774638 [details]
Bug 1285528 - [RTL] Collapse/Expand pane button is not properly displayed;

https://reviewboard.mozilla.org/r/67096/#review64414

::: devtools/client/shared/components/sidebar-toggle.css:27
(Diff revision 2)
>  @media (max-width: 700px) {
> -  .sidebar-toggle::before {
> +  .sidebar-toggle:-moz-locale-dir(ltr)::before {
>      transform: rotate(90deg);
>    }
> +
> +  .sidebar-toggle:-moz-locale-dir(rtl)::before {

I think this is worth adding a comment:
/* Since RTL swaps the used images, we need to flip them the other way round */

	gasolin@mozilla.com Assignee
	Comment 16 • 8 years ago

Comment on attachment 8774638 [details]
Bug 1285528 - [RTL] Collapse/Expand pane button is not properly displayed;

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

	gasolin@mozilla.com Assignee
	Comment 17 • 8 years ago

comment added, thanks for review!

	Pulsebot
	Comment 18 • 8 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/f5875c1a1fbd
[RTL] Collapse/Expand pane button is not properly displayed. r=ntim

	Carsten Book [:Tomcat]
	Comment 19 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f5875c1a1fbd

	Ada [:adalucinet] Reporter
	Comment 20 • 8 years ago

Verified fixed with latest 50.0a1 RTL, under Windows 10 64-bit, Ubuntu 16.04 64-bit and Mac OS X 10.11.

	Itiel
	Comment 22 • 8 years ago

Fixed, but only for the Inspector tab. I guess the same patch should be applied to other tabs (such as Debugger, Network Monitor and possibly more).
Can it be addressed here or should I open a new bug for it?

	Marco Mucci [:MarcoM]
	Comment 23 • 8 years ago

(In reply to ItielMaN from comment #22)
> Fixed, but only for the Inspector tab. I guess the same patch should be
> applied to other tabs (such as Debugger, Network Monitor and possibly more).
> Can it be addressed here or should I open a new bug for it?

Please open a new bug.  Thanks.

	Itiel
	Comment 24 • 8 years ago

bug 1295161 was opened.
Thanks.