Closed Bug 1400266 Opened 2 years ago Closed 2 years ago

Style the bookmarks and history sidebar tree arrows on Linux

Categories

(Firefox :: Theme, defect, P5)

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox63 --- verified

People

(Reporter: Ovidiu, Assigned: Paenglab)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

The sidebar tree doesn't respect the specifications, for example, the arrows from the "Bookmarks Toolbar", "Bookmarks Menu" are not the same as the ones from the spec.

Please see the attachment.
Whiteboard: [photon-visual][triage]
We currently can't style the arrows as specified because of bug 1381453.
Depends on: 1381453
Priority: -- → P5
Summary: The bookmarks and history sidebar doesn't match the specs. on Ubuntu → Style the bookmarks and history sidebar tree arrows on Linux
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
Flags: qe-verify?
Flags: qe-verify? → qe-verify+
QA Contact: ovidiu.boca
Flags: qe-verify+
QA Contact: ovidiu.boca
Whiteboard: [reserve-photon-visual]
Depends on: 1469287
Attached patch Bug1400266- Linux-twisty.patch (obsolete) — Splinter Review
I copied the Windows twisty icons for this patch.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
From a quick look, it seems like this could fix bug 1298020 as well (rtl issues), hence adding a dependency.
Blocks: 1298020
Comment on attachment 8987596 [details] [diff] [review]
Bug1400266- Linux-twisty.patch

Hmm, somehow the r? wasn't applied.
Attachment #8987596 - Flags: review?(dao+bmo)
Comment on attachment 8987596 [details] [diff] [review]
Bug1400266- Linux-twisty.patch

> *  skin/classic/global/in-content/common.css                   (in-content/common.css)
> *  skin/classic/global/in-content/info-pages.css               (in-content/info-pages.css)
>-   skin/classic/global/tree/twisty-clsd.png                    (tree/twisty-clsd.png)
>-   skin/classic/global/tree/twisty-open.png                    (tree/twisty-open.png)
>+  skin/classic/global/tree/twisty-collapsed.svg                (tree/twisty-collapsed.svg)
>+  skin/classic/global/tree/twisty-collapsed-rtl.svg            (tree/twisty-collapsed-rtl.svg)
>+  skin/classic/global/tree/twisty-expanded.svg                 (tree/twisty-expanded.svg)

nit: indentation is off

>+++ b/toolkit/themes/linux/global/tree/twisty-collapsed-rtl.svg
>@@ -0,0 +1,6 @@
>+<!-- This Source Code Form is subject to the terms of the Mozilla Public
>+   - License, v. 2.0. If a copy of the MPL was not distributed with this
>+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>+<svg width="9" height="9" xmlns="http://www.w3.org/2000/svg" stroke="context-fill" stroke-opacity="context-fill-opacity" stroke-width="1.6" fill="none">

Where does fill-opacity ever get set to a value different than 1?

>+    <path d="m 6.5,0.5 -4,4 4,4"/>

nit: use two spaces indentation
Attachment #8987596 - Flags: review?(dao+bmo)
(In reply to Dão Gottwald [::dao] from comment #5)
> Comment on attachment 8987596 [details] [diff] [review]
> Bug1400266- Linux-twisty.patch
> 
> Where does fill-opacity ever get set to a value different than 1?

Nowhere yet. It's only ready if it's maybe used in the sidebar theming bug. Should I remove it completely? Then I'll do the same on Windows.
(In reply to Richard Marti (:Paenglab) from comment #6)
> Should I remove it completely? Then I'll do the same on Windows.

Yes, please.
(In reply to Dão Gottwald [::dao] from comment #5)
> Comment on attachment 8987596 [details] [diff] [review]
> Bug1400266- Linux-twisty.patch
> 
> nit: indentation is off

Fixed.

> Where does fill-opacity ever get set to a value different than 1?

Removed, also on Windows.

> >+    <path d="m 6.5,0.5 -4,4 4,4"/>
> 
> nit: use two spaces indentation

Fixed, also on Windows.
Attachment #8987596 - Attachment is obsolete: true
Attachment #8988768 - Flags: review?(dao+bmo)
Comment on attachment 8988768 [details] [diff] [review]
Bug1400266- Linux-twisty.patch

Thanks!
Attachment #8988768 - Flags: review?(dao+bmo) → review+
Keywords: checkin-needed
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/93972d67be61
Use SVG icons for the twisties on Linux. r=dao
Keywords: checkin-needed
Blocks: 1469287
No longer depends on: 1469287
Blocks: png-cleanup
https://hg.mozilla.org/mozilla-central/rev/93972d67be61
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
I verified this issue on Ubuntu 16.04 with FF Nightly 63.0a1(2018-07-01) and I can confirm the fix. I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
Regressions: 1560540
You need to log in before you can comment on or make changes to this bug.