Closed
Bug 1285528
Opened 9 years ago
Closed 9 years ago
[RTL] Collapse/Expand pane button is not properly displayed
Categories
(DevTools :: General, enhancement, P1)
DevTools
General
Tracking
(firefox47 affected, firefox48 affected, firefox49 affected, firefox50 verified)
People
(Reporter: adalucinet, Assigned: gasolin)
References
(Blocks 1 open bug)
Details
(Whiteboard: [reserve-html][good taipei bug])
Attachments
(1 file)
[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
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [qe-dthtml]
Whiteboard: [devtools-html][triage]
Comment 1•9 years ago
|
||
Hi Alexandra, which bug is this related to?
Flags: needinfo?(alexandra.lucinet)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(alexandra.lucinet)
Summary: [l10n] Collapse/Expand pane button is not properly displayed → [RTL] Collapse/Expand pane button is not properly displayed
Comment 2•9 years ago
|
||
Is there a specific Bug ID I can make this bug block?
Comment 3•9 years ago
|
||
(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]
Reporter | ||
Comment 4•9 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.
Flags: needinfo?(alexandra.lucinet)
Comment 5•9 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.
Blocks: dt-rtl
Priority: -- → P2
Comment 6•9 years ago
|
||
Waiting on team to triage tomorrow.
Severity: normal → enhancement
Priority: P2 → --
Updated•9 years ago
|
Priority: -- → P3
Whiteboard: [devtools-html][triage] → [reserve-html]
Updated•9 years ago
|
Whiteboard: [reserve-html] → [reserve-html][good taipei bug]
Updated•9 years ago
|
Priority: P3 → P2
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•9 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?
Flags: needinfo?(ntim.bugs)
Comment 8•9 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.
Flags: needinfo?(ntim.bugs)
Assignee | ||
Comment 9•9 years ago
|
||
-moz-locale-dir works, thanks! I was thinking the div button is a normal html element :/
Assignee | ||
Comment 10•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/67096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67096/
Attachment #8774638 -
Flags: review?(ntim.bugs)
Comment 11•9 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.
Comment 12•9 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 ?
Attachment #8774638 -
Flags: review?(ntim.bugs)
Assignee | ||
Comment 13•9 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.
Assignee | ||
Comment 14•9 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/
Attachment #8774638 -
Flags: review?(ntim.bugs)
Comment 15•9 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 */
Attachment #8774638 -
Flags: review?(ntim.bugs) → review+
Assignee | ||
Comment 16•9 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/
Comment 18•9 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
Keywords: checkin-needed
Comment 19•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•9 years ago
|
Iteration: --- → 50.4 - Aug 1
Priority: P2 → P1
Reporter | ||
Comment 20•9 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.
Comment 22•9 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?
Comment 23•9 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.
Comment 24•9 years ago
|
||
bug 1295161 was opened.
Thanks.
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•