No treecol sort arrow shown

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
normal
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

({regression})

unspecified
mozilla65
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 unaffected, firefox64 unaffected, firefox65 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

7 months ago
In Library window click on a treecol to change the sort order. No arrow is shown to show the sort order.

This is a regression from bug 1499423.
Assignee

Comment 1

7 months ago
Posted patch 1506705-treecol-sort.patch (obsolete) — Splinter Review
This patch changes the selector to how it is needed now. I also exchanged the PNGs with SVGs. Under Windows 10 the arrows are no more triangles but arrows. For Windows 7 Classic I leaved the old triangle PNGs.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9024573 - Flags: review?(dao+bmo)
Comment on attachment 9024573 [details] [diff] [review]
1506705-treecol-sort.patch

>-  skin/classic/global/tree/sort-asc.png                    (../../windows/global/tree/sort-asc.png)
>-  skin/classic/global/tree/sort-dsc.png                    (../../windows/global/tree/sort-dsc.png)
>+  skin/classic/global/tree/sort-asc.svg                    (../../windows/global/tree/sort-asc.svg)
>+  skin/classic/global/tree/sort-dsc.svg                    (../../windows/global/tree/sort-dsc.svg)

This seems to belong in toolkit/themes/windows/global/jar.mn rather than non-mac.jar.inc.mn.

You'll also need to update browser/base/content/test/static/browser_all_files_referenced.js.

>-treecol:not([hideheader="true"]) > .treecol-sortdirection[sortDirection="ascending"] {
>+treecol[sortDirection="ascending"]:not([hideheader="true"]) > .treecol-sortdirection {

This seems to have the same problem:

https://searchfox.org/mozilla-central/rev/7f7c353e969e61a6a85201cc8ad3c3de12ac30d8/toolkit/themes/shared/in-content/common.inc.css#755

Would you mind fixing this here too?
Attachment #9024573 - Flags: review?(dao+bmo)
Assignee

Comment 3

7 months ago
(In reply to Dão Gottwald [::dao] from comment #2)
> Comment on attachment 9024573 [details] [diff] [review]
> 1506705-treecol-sort.patch
> 
> >-  skin/classic/global/tree/sort-asc.png                    (../../windows/global/tree/sort-asc.png)
> >-  skin/classic/global/tree/sort-dsc.png                    (../../windows/global/tree/sort-dsc.png)
> >+  skin/classic/global/tree/sort-asc.svg                    (../../windows/global/tree/sort-asc.svg)
> >+  skin/classic/global/tree/sort-dsc.svg                    (../../windows/global/tree/sort-dsc.svg)
> 
> This seems to belong in toolkit/themes/windows/global/jar.mn rather than
> non-mac.jar.inc.mn.

Moved.

> You'll also need to update
> browser/base/content/test/static/browser_all_files_referenced.js.
> 
> >-treecol:not([hideheader="true"]) > .treecol-sortdirection[sortDirection="ascending"] {
> >+treecol[sortDirection="ascending"]:not([hideheader="true"]) > .treecol-sortdirection {

I hope removing the lines is the correct way when they are now referenced in toolkit/themes/windows/global/jar.mn

I started a try to see: https://treeherder.mozilla.org/#/jobs?repo=try&revision=380d1f9fb3860af5fc0c85f70d2a7b4483b39c83

> This seems to have the same problem:
> 
> https://searchfox.org/mozilla-central/rev/
> 7f7c353e969e61a6a85201cc8ad3c3de12ac30d8/toolkit/themes/shared/in-content/
> common.inc.css#755
> 
> Would you mind fixing this here too?

Fixed.
Attachment #9024573 - Attachment is obsolete: true
Attachment #9024822 - Flags: review?(dao+bmo)
Comment on attachment 9024822 [details] [diff] [review]
1506705-treecol-sort.patch v2

Looks good, thanks!
Attachment #9024822 - Flags: review?(dao+bmo) → review+
Duplicate of this bug: 1348526
Priority: -- → P1
Assignee

Updated

7 months ago
Keywords: checkin-needed

Comment 6

7 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9725b8b1c14
Fix the treecol sortdirection selectors and use SCG icons for them. r=dao
Keywords: checkin-needed

Comment 7

7 months ago
SCG icons? What's that?

Comment 8

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9725b8b1c14
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.