Last Comment Bug 795342 - Toolbars in Task/Calendar tabs missing due to undefined entities in SeaMonkey.
: Toolbars in Task/Calendar tabs missing due to undefined entities in SeaMonkey.
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Lightning: SeaMonkey Integration (show other bugs)
: Lightning 1.9
: All All
: -- normal (vote)
: 1.9
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on:
Blocks: 785692
  Show dependency treegraph
 
Reported: 2012-09-28 08:33 PDT by Philip Chee
Modified: 2012-10-02 11:09 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.69 KB, patch)
2012-09-28 10:31 PDT, Richard Marti (:Paenglab)
philipp: review+
philip.chee: feedback+
philipp: approval‑calendar‑aurora-
Details | Diff | Review
patch for aurora (2.09 KB, patch)
2012-10-01 11:44 PDT, Richard Marti (:Paenglab)
philipp: review+
philip.chee: review-
Details | Diff | Review

Description Philip Chee 2012-09-28 08:33:53 PDT
Fri Sep 28 2012 23:26:25
Error: undefined entity
Source file: chrome://lightning/content/lightning-toolbar.xul
Line: 108, Column: 7
Source code:
      <toolbarbutton id="calendar-appmenu-button"

The undefined entities come from Bug 785692:

>       <toolbarbutton id="calendar-appmenu-button"
>                      class="toolbarbutton-1 button-appmenu"
>                      label="&appmenuButton.label;"
>                      tooltiptext="&appmenuButton.tooltip;"/>
> 
>       <toolbarbutton id="task-appmenu-button"
>                      class="toolbarbutton-1 button-appmenu"
>                      label="&appmenuButton.label;"
>                      tooltiptext="&appmenuButton.tooltip;"/>
Comment 1 Philip Chee 2012-09-28 08:44:18 PDT
So possible solutions:

1. Using the removeelement trick in suite-overlay-sidebar.xul doesn't work. Or rather it works but comes too late to prevent the undefined entities error.

2. Add these entities to the SeaMonkey messenger.dtd.

3. Move these entities to the Lightning locale file
(lightning-toolbar.dtd).

4. Something else.

Your thoughts Gentlebeings?
Comment 2 Philipp Kewisch [:Fallen] 2012-09-28 09:02:52 PDT
Paenglab, can you take care of this bug?
Comment 3 Stefan Sitter 2012-09-28 09:34:47 PDT
If I read the other bugs correctly this landed on comm-aurora, i.e. Lightning 1.9a2 and SeaMonkey 2.14a2 are affected?
Comment 4 Richard Marti (:Paenglab) 2012-09-28 10:31:51 PDT
Created attachment 665964 [details] [diff] [review]
patch

I took option 3 as it's the easiest and doesn't add entitys to SM which aren't used there and could be removed later because of 'not used' in SM.

Philip, can you test the patch under SM if it solves this issue?
Comment 5 Richard Marti (:Paenglab) 2012-09-29 10:19:26 PDT
Comment on attachment 665964 [details] [diff] [review]
patch

Bug 785692 has also approval for aurora. Then it makes sense to push this patch also to aurora.
Comment 6 Philipp Kewisch [:Fallen] 2012-09-29 10:48:35 PDT
Comment on attachment 665964 [details] [diff] [review]
patch

Problem is this changes strings. Even though its the same known string, it is still a string change. If we want to push this to aurora, we will have to notify localizers using various channels.

If there is a different solution we can use on aurora only (even if its a hack like copying the string via JS), I would prefer that.
Comment 7 Richard Marti (:Paenglab) 2012-09-29 11:15:55 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> Problem is this changes strings. Even though its the same known string, it
> is still a string change. If we want to push this to aurora, we will have to
> notify localizers using various channels.
> 
> If there is a different solution we can use on aurora only (even if its a
> hack like copying the string via JS), I would prefer that.

Maybe Philip has a solution.
Comment 8 Philip Chee 2012-09-30 13:08:43 PDT
> Maybe Philip has a solution.
In lightning-toolbar.xul do something like:

<!DOCTYPE overlay [
  <!ENTITY % messengerDTD SYSTEM "chrome://messenger/locale/messenger.dtd"> %messengerDTD;
  <!ENTITY % mailOverlayDTD SYSTEM "chrome://messenger/locale/mailOverlay.dtd"> %mailOverlayDTD;
  <!ENTITY % menuOverlayDTD SYSTEM "chrome://calendar/locale/menuOverlay.dtd" > %menuOverlayDTD;
  <!ENTITY % lightningDTD SYSTEM "chrome://lightning/locale/lightning.dtd"> %lightningDTD;
  <!ENTITY % calendarDTD SYSTEM "chrome://calendar/locale/calendar.dtd" > %calendarDTD;
  <!ENTITY % toolbarDTD SYSTEM "chrome://lightning/locale/lightning-toolbar.dtd" > %toolbarDTD;
  <!ENTITY label "AppMenu">
  <!ENTITY appmenuButton.tooltip "Displays the Application Menu">
]>

In TB the entities are satisfied by messenger.dtd so the last two items I've just added aren't used. The SM messenger.dtd doesn't have those entities so processing will fall through to the entites at the bottom.

This means that in SeaMonkey the App Menu toolbar buttons get hard coded English strings, but that's OK since SeaMonkey doesn't have an App Menu widget. To make sure that the toolbar buttons don't appear in SeaMonkey we can add a couple of lines to suite-overlay-sidebar.xul like:

      <toolbarbutton id="calendar-appmenu-button" removeelement="true"/>
      <toolbarbutton id="task-appmenu-button" removeelement="true"/>
Comment 9 Philip Chee 2012-10-01 03:54:57 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/404b3c94f8b7

(sorry don't know what the target milestone should be set to)
Comment 10 Richard Marti (:Paenglab) 2012-10-01 11:42:13 PDT
Philip, this changeset is from Bug 785580. This patch isn't checked in. It's still awaiting your feedback.
Comment 11 Richard Marti (:Paenglab) 2012-10-01 11:44:06 PDT
Created attachment 666635 [details] [diff] [review]
patch for aurora

Patch for aurora with the entities directly in the XUL file for SM.
Comment 12 Philipp Kewisch [:Fallen] 2012-10-02 02:31:08 PDT
Comment on attachment 666635 [details] [diff] [review]
patch for aurora

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

::: calendar/lightning/content/lightning-toolbar.xul
@@ +10,5 @@
>    <!ENTITY % lightningDTD SYSTEM "chrome://lightning/locale/lightning.dtd"> %lightningDTD;
>    <!ENTITY % calendarDTD SYSTEM "chrome://calendar/locale/calendar.dtd" > %calendarDTD;
>    <!ENTITY % toolbarDTD SYSTEM "chrome://lightning/locale/lightning-toolbar.dtd" > %toolbarDTD;
> +  <!ENTITY lightning.toolbar.appmenuButton.label "AppMenu">
> +  <!ENTITY lightning.toolbar.appmenuButton.tooltip "Displays the Application Menu">

Maybe you could add a comment describing why its needed.
Comment 13 Philip Chee 2012-10-02 10:34:46 PDT
Comment on attachment 665964 [details] [diff] [review]
patch

This is a GO! Thanks for the patch. I'll look into the aurora patch in a bit.
Comment 14 Richard Marti (:Paenglab) 2012-10-02 10:51:36 PDT
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/7440e1d7892a
Comment 15 Philip Chee 2012-10-02 10:53:02 PDT
Comment on attachment 666635 [details] [diff] [review]
patch for aurora

-                     label="&appmenuButton.label;"
-                     tooltiptext="&appmenuButton.tooltip;"/>
+                     label="&lightning.toolbar.appmenuButton.label;"
+                     tooltiptext="&lightning.toolbar.appmenuButton.tooltip;"/>

Oops I may have misled you. You need to keep the entities the same on aurora so that on Thunderbird you pick up the appmenuButton.* entities from messenger.dtd but on SeaMonkey from the hard coded entities in lightning-toolbar.xul

So this becomes:

+  <!ENTITY appmenuButton.label "AppMenu">
+  <!ENTITY appmenuButton.tooltip "Displays the Application Menu">

And the following changes are not needed:

-                     label="&appmenuButton.label;"
-                     tooltiptext="&appmenuButton.tooltip;"/>
+                     label="&lightning.toolbar.appmenuButton.label;"
+                     tooltiptext="&lightning.toolbar.appmenuButton.tooltip;"/>

f+ me with this change.
Comment 16 Richard Marti (:Paenglab) 2012-10-02 11:09:34 PDT
Pushed only the entities with a comment to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/a1b5c0435667

Note You need to log in before you can comment on or make changes to this bug.