[RTL] Collapse/Expand pane button is not properly displayed

VERIFIED FIXED in Firefox 50

Status

enhancement
P1
normal
VERIFIED FIXED
3 years ago
Last year

People

(Reporter: adalucinet, Assigned: gasolin)

Tracking

(Blocks 1 bug)

Trunk
Firefox 50
Dependency tree / graph

Firefox Tracking Flags

(firefox47 affected, firefox48 affected, firefox49 affected, firefox50 verified)

Details

(Whiteboard: [reserve-html][good taipei bug])

Attachments

(1 attachment)

[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
QA Whiteboard: [qe-dthtml]
Whiteboard: [devtools-html][triage]
Hi Alexandra, which bug is this related to?
Flags: needinfo?(alexandra.lucinet)
Flags: needinfo?(alexandra.lucinet)
Summary: [l10n] Collapse/Expand pane button is not properly displayed → [RTL] Collapse/Expand pane button is not properly displayed
Is there a specific Bug ID I can make this bug block?
(In reply to Marco Mucci [:MarcoM] from comment #2)
> Is there a specific Bug ID I can make this bug block?
Flags: needinfo?(alexandra.lucinet)
Whiteboard: [devtools-html][triage] → [devtools-html][triage]
(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.
Flags: needinfo?(alexandra.lucinet)
(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.
Flags: qe-verify+
QA Contact: alexandra.lucinet
Priority: -- → P2
Waiting on team to triage tomorrow.
Severity: normal → enhancement
Priority: P2 → --
Priority: -- → P3
Whiteboard: [devtools-html][triage] → [reserve-html]
Whiteboard: [reserve-html] → [reserve-html][good taipei bug]
Priority: P3 → P2
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
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?
Flags: needinfo?(ntim.bugs)
(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.
Flags: needinfo?(ntim.bugs)
-moz-locale-dir works, thanks! I was thinking the div button is a normal html element :/
(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.
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 ?
Attachment #8774638 - Flags: review?(ntim.bugs)
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.
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/
Attachment #8774638 - Flags: review?(ntim.bugs)
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 */
Attachment #8774638 - Flags: review?(ntim.bugs) → review+
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/
comment added, thanks for review!
Keywords: checkin-needed
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f5875c1a1fbd
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Iteration: --- → 50.4 - Aug 1
Priority: P2 → P1
Verified fixed with latest 50.0a1 RTL, under Windows 10 64-bit, Ubuntu 16.04 64-bit and Mac OS X 10.11.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1205556
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?
(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.
bug 1295161 was opened.
Thanks.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.