Port bug 1471403 - Migrate <notificationbox> to a JS class

RESOLVED FIXED in Thunderbird 65.0

Status

defect
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: jorgk, Assigned: mkmelin)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 65.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
See bug 1471403 comment #16 (quote):

... the API for <notificationbox> is going to change. Rather than going from XBL -> Custom Element it's going from XBL -> JS class that builds the DOM. And the element will only contain notifications instead of containing notifications and children. Parts 1 and Part 5 are probably the most relevant changesets to TB. Alternatively you can temporarily restore the <notification> and <notificationbox> bindings and continue to bind them to the DOM nodes.
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)
(Reporter)

Updated

6 months ago
Summary: Port bug 1471403 - Migrate <notificationbox> to a Custom Element → Port bug 1471403 - Migrate <notificationbox> to a JS class or copy binding
(Assignee)

Comment 1

6 months ago
Well for better or worse, the notificationbox binding is already copied at https://searchfox.org/comm-central/source/common/bindings/notificationbox.xml

... due to Bug 1487087 - Restore the button[type="menu-button"] bindings which were removed in bug 1457218.

So I would guess not directly pending bustage, but we should look into it.
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)
Summary: Port bug 1471403 - Migrate <notificationbox> to a JS class or copy binding → Port bug 1471403 - Migrate <notificationbox> to a JS class
(Reporter)

Comment 2

6 months ago
Well, here is your bustage:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=211048341&revision=37233a992def692093f85e6b88860bc500869634

TEST-UNEXPECTED-FAIL | Z:\task_1541891382\build\tests\mozmill\testBasicFunctionality.js | testBasicFunctionality.js::testSmokeTest
TEST-UNEXPECTED-FAIL | Z:\task_1541891382\build\tests\mozmill\testLocalICS.js | testLocalICS.js::testLocalICS

https://taskcluster-artifacts.net/P1L_TaiMReyp_W8bbwq_xQ/0/public/logs/live_backing.log
Heaps of
JavaScript error: chrome://calendar/content/calendarCreation.js, line 121: TypeError: notificationbox.removeAllNotifications is not a function

Equally, various tests related to notifications fail now:
cloudfile\test-cloudfile-notifications.js
composition\test-attachment-reminder.js
content-tabs\test-lwthemes.js

The cloudfile and attachment reminder stuff fails with
EXCEPTION: this.closest(...) is null; can't access its "_notificationBox" property,
the LW themes test is less obvious:
EXCEPTION: currentLwTheme(...) is undefined; can't access its "id" property
EXCEPTION: mc.tabmail.selectedTab.panel is undefined; can't access its "querySelector" property
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)
(Reporter)

Comment 3

6 months ago
Actually, the LW theme issue may come from bug 1506411 (bug 1506411 comment #3). We'll see whether it's fixed now.
(Assignee)

Comment 4

6 months ago
Seems to do it.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(mkmelin+mozilla)
Attachment #9024282 - Flags: review?(jorgk)
(Reporter)

Comment 5

6 months ago
Thanks, that will fix all tests? Let's see:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a36a6a2ac9d6d09275f6f0b821bc2a5531bb3059
Flags: needinfo?(geoff)
(Reporter)

Comment 6

6 months ago
Comment on attachment 9024282 [details] [diff] [review]
bug1505288_notificationbox_js.patch

Thanks. I guess the code is copied from somewhere, so I don't have to read it line by line.

That hasn't fixed the LW Theme failures, so I need to file a new bug for it.
Attachment #9024282 - Flags: review?(jorgk) → review+

Comment 7

6 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d55eedb42e08
adapt comm-central to notification box for changes in bug 1471403 (Migrate <notificationbox> to a JS class). r=jorgk
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
(Reporter)

Updated

6 months ago
Target Milestone: --- → Thunderbird 65.0
(Reporter)

Comment 8

6 months ago
Filed bug 1506470 for the LW theme failures.
(Assignee)

Comment 9

6 months ago
(In reply to Jorg K (GMT+1) from comment #6)
> I guess the code is copied from somewhere, 

yeah direct copy of what <notification> was in toolkit before the change.
You need to log in before you can comment on or make changes to this bug.