Closed Bug 1506705 Opened 5 years ago Closed 5 years ago

No treecol sort arrow shown

Categories

(Toolkit :: Themes, defect, P1)

All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
Attached 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)
(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+
Priority: -- → P1
Keywords: checkin-needed
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
SCG icons? What's that?
https://hg.mozilla.org/mozilla-central/rev/b9725b8b1c14
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: