Style the bookmarks and history sidebar tree arrows on Linux

VERIFIED FIXED in Firefox 63

Status

()

P5
normal
VERIFIED FIXED
a year ago
5 months ago

People

(Reporter: Ovidiu, Assigned: Paenglab)

Tracking

(Blocks: 2 bugs)

57 Branch
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8908624 [details]
sidebar tree Ubuntu 16.04.png

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

Updated

a year ago
Blocks: 1325171
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
(Assignee)

Comment 2

7 months ago
Created attachment 8987596 [details] [diff] [review]
Bug1400266- Linux-twisty.patch

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

Comment 4

7 months ago
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)
(Assignee)

Comment 6

7 months ago
(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.
(Assignee)

Comment 8

7 months ago
Created attachment 8988768 [details] [diff] [review]
Bug1400266- Linux-twisty.patch

(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+
(Assignee)

Updated

7 months ago
Keywords: checkin-needed

Comment 10

7 months ago
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

Updated

7 months ago
Blocks: 1450569

Comment 11

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/93972d67be61
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
(Reporter)

Comment 12

7 months ago
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
status-firefox63: fixed → verified
status-firefox57: affected → ---
You need to log in before you can comment on or make changes to this bug.