Closed Bug 1507245 Opened 6 years ago Closed 6 years ago

Adapt notification binding to changes in bug 1496827

Categories

(Thunderbird :: General, enhancement)

Product:
Thunderbird
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 65.0

People

(Reporter: jorgk-bmo, Assigned: arshad)

References

Details

Attachments

(2 files, 1 obsolete file)

double-notification.png
2.74 KB, image/png
Details
xnotification.patch
15.72 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review 
See bug 1496827 comment #7 and above:

You'll probably need to rename the XBL'ed element from <notification> to something else so that you don't end up with both XBL and a Custom Element attached to it after this bug lands.

Bug 1496827 is on inbound, so we'll have bustage soon :-(
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)
Ok, I am on this. What should I rename the xbl notification element to?
Flags: needinfo?(arshdkhn1)
notificationXBL? ;-) legacy_notification?
Something lowercase. Perhaps xbl-notification.
Flags: needinfo?(mkmelin+mozilla)
Attached patch xnotification.patch (obsolete) — — Splinter Review 
I have renamed all the instances of notification to xbl-notification. The css might be off because the notfication css that comes from /mozilla hasn't been copied yet. What should I do with the notification css coming from /mozilla? Should I copy it to a file add it to messenger.css?
Assignee: nobody → arshdkhn1
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9025207 [details] [diff] [review]
xnotification.patch

Looks good.

You need to copy the notification rules from notification.inc.css. I propose to create a new file like xbl-notification.css in our themes/shared and @import it in bindings.css. Like this it's easy to remove again when the XBL is no more needed.
Flags: needinfo?(richard.marti)
[Richard already answered]
Flags: needinfo?(mkmelin+mozilla)
No test failures to be seen, does that mean that everything is still working?
Possibly, dunno if the custom element or xbl wins when both are applied, or if the element gets into some kind of Frankenstein state with both behaviours. At least some css might have to be adjusted for things to look like it should.
Attached image double-notification.png — 
This is how it looks actually.
I am so cheeky and and add the needed CSS. I also removed the no more needed rules for mac as we have now a popup and moved the messenger.css rules to xbl-notification.css.
Attachment #9025306 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9025306 [details] [diff] [review]
xnotification.patch

Review of attachment 9025306 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin
Attachment #9025306 - Flags: review?(mkmelin+mozilla) → review+
Attachment #9025207 - Attachment is obsolete: true
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3f6a3f235390
Rename xbl notification element to xbl-notification. r=mkmelin
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 65.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: