551317 - entities saying "Browser" and "page" in EditorColorProperties.dtd in the message composer

Status: ASSIGNED

(MailNews Core :: Composition, defect, P5)

Description

[Adrian Kalla [:adriank]]

(at least) the following three entities are being used in Thunderbird (message compose) and SeaMonkey (composer), but their meaning is wrong for at least one of the applications:

http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#41
http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#43
http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#46

The text there should be changed to be application neutral (no "browsers", "readers" or "pages"), or there should be separate entities for each of the applications.

This bug has been filled as a follow up from the Aviary.pl bug 2169: http://bugs.aviary.pl/show_bug.cgi?id=2169

Comment 1

[:aceman]

The line numbers referenced do not exist anymore. Can you please see if this bug is still existing and update the links?

Comment 2

[Bartosz Piec]

This bus is about the following entities:
‒ defaultColorsRadio.label (http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#8)
‒ defaultColorsRadio.tooltip (http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#10)
‒ customColorsRadio.tooltip (http://mxr.mozilla.org/comm-central/source/editor/ui/locales/en-US/chrome/dialogs/EditorColorProperties.dtd#13)

and they're still in the source.

Comment 3

[:aceman]

Thanks, I see the problem now. This dialog is shown when composing a message but talks about setting background of a page.

But it is also used in the Seamonkey Composer (webpage creator tool). There the "page" is correct.

Maybe changing "page" to "document" would be fine. I'll make some proposal patch.

Comment 4

[:aceman]

Attachment: patch

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 668837 [details] [diff] [review]
patch

The Thunderbird menu is still titled "Page Colors and Background…", probably from /editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd.
(http://mxr.mozilla.org/comm-central/search?string=Page%20Colors should have all the occurrences you want to change.)

But that's pretty minor, so ui-r=me with that fixed.

Thanks,
Blake.

Comment 6

[:aceman]

Attachment: patch v2

I wonder if this works in Seamonkey too.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 672029 [details] [diff] [review]
patch v2

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

Looks ok to me. r=mkmelin

Comment 8

[Ian Neal]

Comment on attachment 672029 [details] [diff] [review]
patch v2

>+++ b/editor/ui/composer/content/editorOverlay.xul

>   <menuitem id="colorsAndBackground"
>-            label="&colorsAndBackground.label;"
>-            accesskey="&colorsAndBackground.accesskey;"
Instead of removing these completely move them to the menuitem in editor.xul:
http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.xul#192
and move the entities from editorOverlay.dtd to editor.dtd

>+            label="&documentColorsAndBackground.label;"
>+            accesskey="&documentColorsAndBackground.accesskey;"
Then add these two attributes to the menuitem in messengercompose.xul:
http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/messengercompose.xul#567
http://mxr.mozilla.org/comm-central/source/suite/mailnews/compose/messengercompose.xul#307
and add the new entities to the two messengercompose.dtd files.

I'm not comfortable with the changes for EdColorProps.xul
I think it is a bit messy with viewer / reader, etc.
Need to find a way that for mail it can talk about message, recipient, etc and for composer it can talk about page, viewer, etc.

Comment 9

[:aceman]

You have now moved the logic about mail/page out to /mail & /suite but the EdColorProps.xul stays in /editor. Can we somehow overlay it too?

Comment 10

[Ian Neal]

(In reply to :aceman from comment #9)
> You have now moved the logic about mail/page out to /mail & /suite but the
> EdColorProps.xul stays in /editor. Can we somehow overlay it too?

There might be a way of overlaying it, but probably will depend on bug 700808

Comment 11

[:aceman]

So what now?

Comment 12

[Wayne Mery (:wsmwk)]

Do we still care about this?

Comment 13

[Rob Lemley [:rjl]]

The bug still exists and it looks like all of the code referencing that string is in comm-central now so it's very fixable.

Comment 14

[Magnus Melin [:mkmelin]]

Right. Very low priority.