Closed Bug 1604155 Opened 5 years ago Closed 4 years ago

replace XULElement.ordinal property usage in Thunderbird

Categories

(Calendar :: Tasks, task, P1)

Product:
Calendar
Component:
Tasks
Type:
task

Tracking

(thunderbird78 unaffected)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 74.0
Tracking Flags:
Tracking Status
thunderbird78 --- unaffected

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

(Whiteboard: [thunderbird-disabled-test])

Attachments

(2 files, 1 obsolete file)

bug1604155_replace_ordinal.patch
2.18 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
bug1604155_replace_xulelement_ordinal_tb.patch
11.95 KB, patch
darktrojan
: feedback+
Details | Diff | Splinter Review

+++ This bug was initially created as a clone of Bug #1603830 +++

Bug 1603830 is getting rid of XULElement.ordinal. The replacement is still under discussion.

Turns out only calendar needs change.

Status: NEW → ASSIGNED
Component: General → Tasks
Priority: -- → P1
Product: Thunderbird → Calendar
Target Milestone: --- → 73

In calendar/base/content/calendar-task-tree.js

Status: ASSIGNED → NEW
Attached patch bug1604155_replace_ordinal.patch (obsolete) — — Splinter Review
Attachment #9118673 - Flags: review?(geoff)
Status: NEW → ASSIGNED
Summary: replace XULElement.ordinal usage in Thunderbird → replace XULElement.ordinal property usage in Thunderbird
Comment on attachment 9118673 [details] [diff] [review] bug1604155_replace_ordinal.patch I don't see the need to use Number() as setAttribute only takes a string, but perhaps it's for converting "" to 0.
Attachment #9118673 - Flags: review?(geoff) → review+

Without Number(). I agree that seems redundant.

Attachment #9118673 - Attachment is obsolete: true
Attachment #9118933 - Flags: review+
Keywords: checkin-needed-tb

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/1aff261d2e16
replace XULElement.ordinal property usage in Thunderbird. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED

This change wasn't quite correct. But the main change looks also not working for treecols: filed bug 1607432.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/ca3bda2ded04
temporarily disable browser_columns.js. r=bustage-fix

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED

Sigh, proper targets missing here, should be 74.

Flags: needinfo?(philipp)
Flags: needinfo?(geoff)

(Was going to continue in this bug)

Status: RESOLVED → REOPENED
Flags: needinfo?(philipp)
Flags: needinfo?(geoff)
Keywords: leave-open
Resolution: FIXED → ---
Whiteboard: [thunderbird-disabled-test]

You'll still need the 74 milestone at some stage, so why not leave the NI? 73 is certainly wrong.

Target Milestone: 73 → 74

This works, kind of. Works fine in the UI, but the test is acting up.

The tree has some very odd timing(?) issue. This passes locally, but not when running --headless.

Any ideas?

Attachment #9125513 - Flags: feedback?(geoff)
Comment on attachment 9125513 [details] [diff] [review] bug1604155_replace_xulelement_ordinal_tb.patch I suggest changing `assert_visible_columns` to wait for the right result instead of expecting it immediately. Everything seems to pass if you wait long enough in the right places.
Attachment #9125513 - Flags: feedback?(geoff) → feedback+

Can this be marked as resolved with the fix in bug 1612055? Maybe also the skipped test file from comment#8 can be re-enabled.

Flags: needinfo?(mkmelin+mozilla)
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/ed7598393624 re-enable browser_columns.js which is working again after bug 1612055. rs=me

Thanks, this can indeed now be closed.

Status: REOPENED → RESOLVED
Closed: 5 years ago → 4 years ago
Flags: needinfo?(mkmelin+mozilla)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: