1914085 - Calendar Tab Event Search Results Sort Indicator Missing

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Mark Barnes [:WoofGrrrr]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:129.0) Gecko/20100101 Firefox/129.0

Steps to reproduce:

128.1.0esr on Windows 11 Pro

1. Open the Calendar Tab
2. Select "Events in this Calendar Month"
3. Click on the "Start" Table Header

Actual results:

The Table gets sorted by Start Date, but there is no indicator sort direction indicator.

Expected results:

There should be an icon to the right of the label to indicate the sort direction.

I opened the Developer Toolbox Inspector to look at things. I think that this is what's happening...

The column header is created using this XUL:

<treecol xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
id="unifinder-search-results-tree-col-startdate"
persist="hidden ordinal width"
style="flex: 1 1 auto; order: 3; width: 163px;"
closemenu="none"
itemproperty="startDate"
data-l10n-id="calendar-event-listing-column-start-date"
label="Start"
tooltiptext="Sort by start date"
ordinal="3"
width="163">
<label class="treecol-text" flex="1" crop="end" value="Start"/>
<image class="treecol-sortdirection"/>
</treecol>

The CSS that should place the sort indicator is at line #264 of tree.css:

/* ::::: sort direction indicator ::::: */
.treecol-sortdirection {
list-style-image: url("chrome://global/skin/tree/sort-asc.svg");
-moz-context-properties: fill;
fill: currentColor;
visibility: hidden;

treecol[sortDirection="ascending"]:not([hideheader="true"]) > & {
visibility: inherit;
}

treecol[sortDirection="descending"]:not([hideheader="true"]) > & {
visibility: inherit;
list-style-image: url("chrome://global/skin/tree/sort-dsc.svg");
}
}

NOTICE that the <treecol> element in the XUL above is completely missing the "sortDirection" attribute, so the <image> with class "treecol-sortdirection" still has the CSS "visibility: hidden" applied to it.

Now I'm not 100% sure how nested CSS like that above is supposed to work. Specifically, I don't understand something like this:

.treecol-sortdirection { treecol > & {xxx} }

I would think that a <treecol> would have to be nested INSIDE an element with class "treecol-sortdirection" for the CSS that enables the sort indicator to match.

But I edited the XML to add attribute "sortDirection='ascending'" to the <treecol> and it fixed the problem. Except there's a bit more to it. Every time I tried to add "sortDirection" the inspector lower-cased it to "sortdirection". I tried both editing the element inline and using the pop-up menu Attributes -> Add Attribute. So I had to edit the CSS in tree.css to change "sortDirection" to "sortdirection". Bingo!

Comment 1

[Mark Barnes [:WoofGrrrr]]

sorry about the messed up formatting

Comment 2

[Magnus Melin [:mkmelin]]

Did this use to work properly in 115?

Comment 3

[Richard Marti (:Paenglab)]

115 is okay. Did some tests and 2023-09-28 is okay but 2023-09-29 not. https://hg.mozilla.org/comm-central/rev/15e6413835fc looks suspicious.

Comment 4

[Magnus Melin [:mkmelin]]

Yeah from bug 1763752

Comment 5

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #4)

Yeah from bug 1763752

Geoff any insight?

Comment 6

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1914085 - Fix sort direction indicator in Find Events pane. r=#thunderbird-reviewers

Comment 7

[Geoff Lankow (:darktrojan)]

Yeah, blame me, I broke it.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f713d4941355
Fix sort direction indicator in Find Events pane. r=aleca

Comment 9

[Geoff Lankow (:darktrojan)]

Comment on attachment 9463669 [details]
Bug 1914085 - Fix sort direction indicator in Find Events pane. r=#thunderbird-reviewers

[User impact if declined]

• Sort indicators are missing on the events list.

[Is this code covered by automated tests?]

• No.

[Has the fix been verified in Daily? (or Beta for an ESR uplift?)]

• Yes. I have checked it works.

[Needs manual test from QA?]

• No.

[List of other uplifts needed]

[Risk to taking this patch]

• Low. This is a simple patch.

[Why is the change risky/not risky? (and alternatives if risky)]

• This question seems redundant?

[String changes made/needed]

• None.

Comment 10

[Corey Bryant]

Comment on attachment 9463669 [details]
Bug 1914085 - Fix sort direction indicator in Find Events pane. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Comment 11

[Corey Bryant]

Thunderbird 136.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/49649940a680

Comment 12

[Mark Barnes [:WoofGrrrr]]

I just tried this out on 138.0a1 (today's build).

I seems to work differently from the way it did before.

My CSS was selecting on [sortActive="true"] but this fix doesn't seem to add that back. So I've had to change it to [sortDirection]

Comment 13

[Geoff Lankow (:darktrojan)]

Comment on attachment 9463669 [details]
Bug 1914085 - Fix sort direction indicator in Find Events pane. r=#thunderbird-reviewers

[User impact if declined]

• Sort indicators are missing on the events list.

[Is this code covered by automated tests?]

• No.

[Has the fix been verified in Daily? (or Beta for an ESR uplift?)]

• Yes. I have checked it works.

[Needs manual test from QA?]

• No.

[List of other uplifts needed]

[Risk to taking this patch]

• Low. This is a simple patch.

[Why is the change risky/not risky? (and alternatives if risky)]

[String changes made/needed]

• None.

Comment 14

[Corey Bryant]

Comment on attachment 9463669 [details]
Bug 1914085 - Fix sort direction indicator in Find Events pane. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr128

Comment 15

[Daniel Darnell [:dandarnell]]

Thunderbird 128.9.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/209429878314

Comment 16

[Mark Barnes [:WoofGrrrr]]

Thank you!