1320442 - Port notification (doorhanger) changes from Bug 1315236 to Seamonkey

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Frank-Rainer Grahl (:frg)]

In toolkit/locales/en-US/chrome/global/notification.dtd

string variables were added which. SeaMonkey currently uses a copy of this file (why its identical?) in 

suite\locales\en-US\chrome\common\notification.dtd

The new variables need to be added or you will see erros in the log and add-on notifications do work.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: 1320442-NotificationStrings.patch

I looked at suite\common\bindings\notification.xml and don't think anything else is needed but could you check.

Comment 2

[Frank-Rainer Grahl (:frg)]

Error without patch:

Timestamp: 11/26/2016 12:02:38 PM
Error: XML Parsing Error: undefined entity
Location: jar:file:///C:/Program%20Files/SeaMonkey/omni.ja!/chrome/toolkit/content/global/bindings/notification.xml
Line Number 531, Column 9:
Source File: jar:file:///C:/Program%20Files/SeaMonkey/omni.ja!/chrome/toolkit/content/global/bindings/notification.xml
Line: 531, Column: 9
Source Code:
        <xul:button anonid="button"

Comment 3

[Philip Chee]

Comment on attachment 8814548 [details] [diff] [review]
1320442-NotificationStrings.patch

(In reply to Frank-Rainer Grahl from comment #0)
> In toolkit/locales/en-US/chrome/global/notification.dtd
> 
> string variables were added which. SeaMonkey currently uses a copy of this
> file (why its identical?) in 
> 
> suite\locales\en-US\chrome\common\notification.dtd
> 
> The new variables need to be added or you will see erros in the log and
> add-on notifications do work.

The right thing to do is to remove the override in /suite/locales/jar.mn:

% override chrome://global/locale/notification.dtd chrome://communicator/locale/notification.dtd 

And then edit /suite/common/bindings/notification.xml
Change:

<!DOCTYPE bindings [
<!ENTITY % notificationDTD SYSTEM "chrome://global/locale/notification.dtd">
%notificationDTD;
]>

to

<!DOCTYPE bindings [
<!ENTITY % communicatorDTD SYSTEM "chrome://communicator/locale/notification.dtd">
%communicatorDTD;
<!ENTITY % globalDTD SYSTEM "chrome://global/locale/notification.dtd">
%globalDTD;
]>

This way anything missing in chrome://communicator/locale/notification.dtd will fall through to the toolkit version and we can stop chasing toolkit.

See how /suite/common/bindings/prefwindow.xml does it as an example. (The formatting in prefwindow makes things clearer at the expense of line length.

> <!-- LOCALIZATION NOTE (closeNotificationItem.label): This entity is used
>      to replace the Toolkit default closeNotificationItem.label ("Not Now")
>      which is ambiguous in some cases. Thus, make sure to select a phrase
>      that clearly relates to closing the current doorhanger. -->

We should also find out where the toolkit "Not Now" string went since &closeNotificationItem.label; doesn't seem to exist in mozilla-central any more.

Comment 4

[Frank-Rainer Grahl (:frg)]

Attachment: Capture.PNG

>> We should also find out where the toolkit "Not Now" string went since 
>> &closeNotificationItem.label; doesn't seem to exist in mozilla-central any more.

Good catch. I didn't notice this one. It was removed in Bug 1282768. I think Bug 1267604 mentioned there is why the add-on notifications now look so ugly.

Comment 5

[Frank-Rainer Grahl (:frg)]

rsx11m,

could you look at this one. Still not firm with xbl and css and it seems to properly fix it 3 bugs need to be ported/merged. 

Otherwise can do it later but would like to poke at 2 other bugs first which affect 2.49.

Comment 7

[Rainer Bielefeld]

Bug 1322824 (both observed issues) is NOT (no longer) reproducible with Server-Installation of  unofficial (by FRG) en-US SeaMonkey 2.50a1 (NT 6.1; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 Build 20161211141438  (Default Classic Theme) on German WIN7 64bit

Comment 8

[Philip Chee]

Attachment: Patch v1 don't override toolkit notification.dtd instead use both in notification.xml

WIP patch. Will spin off a new bug or bugs to port Bug 1282768 and Bug 1267604

Comment 9

[Philip Chee]

Attachment: Fixed Patch v1.1 don't override toolkit notification.dtd instead use both in notification.xml

Working patch. Will spin off a new bug or bugs to port Bug 1282768 and Bug 1267604

Comment 10

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8818267 [details] [diff] [review]
Fixed Patch v1.1 don't override toolkit notification.dtd instead use both in notification.xml

errors in the log are gone with the patch.

Comment 12

[Tony Mechelynck [:tonymec]]

IIUC, this is the bug for the recent "empty doorhangers" regression, a nasty one IMO.

Comment 13

[Philip Chee]

Pushed to comm-central a=bustage fix
http://hg.mozilla.org/comm-central/rev/f3286b40ab87
http://hg.mozilla.org/comm-central/rev/f3286b40ab87aaa39de0d5d1b97ee2b52cfd6f2f

Comment 14

[Philip Chee]

(In reply to Philip Chee from comment #9)
> Working patch. Will spin off a new bug or bugs to port Bug 1282768 and Bug 1267604

I filed Bug 1323494 for these.

Comment 15

[Tony Mechelynck [:tonymec]]

There must be something else (CSS variables?) I still see empty doorhangers (in about:addons, which is where I usually see doorhangers)

UA:"Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0 SeaMonkey/2.52a1" 
ID:20170310003001 en-US 
c-c:245528202a53a82b14d1a17fa5458d23d6cc6c07 
m-c:35398cae65c1526ce45c23a5f8b5568c5ada4762