1039933 - While composing mail, there exists an empty line in attachment context menu when right clicked on the attachment area

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, defect)

Description

[Mehmet Tahir SANDIKKAYA]

Attachment: bug1.jpg

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 SeaMonkey/2.26.1 (Beta/Release)
Build ID: 20140612174402

Steps to reproduce:

1. Start composing new mail.
2. Right click on the attachment area.


Actual results:

Attachment context menu appears with an empty line.


Expected results:

There must not be an empty line.

Comment 1

[Philip Chee]

Regression:
Bug 650885 - Make better use of menuitem and command elements relating to editing already in utilityOverlay

http://hg.mozilla.org/comm-central/rev/b44563195827#l13.19

Hg Blame says IanN.

Comment 2

[Ian Neal]

I do not see this on latest trunk on linux, there is no empty line.
Is this reproducible for latest trunk on other platforms?

Comment 3

[Philip Chee]

(In reply to Ian Neal from comment #2)
> I do not see this on latest trunk on linux, there is no empty line.
Well there *should* be a line, ideally with the value of "Select All". And in fact it's there if you use the DOMi to look at the context menu.

> Is this reproducible for latest trunk on other platforms?
It's there in Windows. The line height might be 0 due to not having a label which you deleted.

> -      <menuitem label="&selectAll.label;"
> -                accesskey="&selectAll.accesskey;"
> +      <menuitem accesskey="&selectAllAttachments.accesskey;"
>                  command="cmd_selectAll"/>

I'm not sure why you think we need a menuitem with no label but with an access key.

Comment 4

[Philip Chee]

Attachment: Patch v1.0 Fix the missing label.

Fix missing label for "Select All" attachment area context menu.

Comment 5

[Ian Neal]

Comment on attachment 8466308 [details] [diff] [review]
Patch v1.0 Fix the missing label.

Rather than adding a new entity, can we not just use selectAllCmd.label from utilityOverlay.dtd?
Plus that is what my local patch had in, just not had chance to do anything with it as I got distracted by build issues.
Also has the advantage of being usable for aurora and beta.

Comment 6

[Philip Chee]

Attachment: Patch v2.0 Branch safe patch.

(In reply to Ian Neal from comment #5)
> Rather than adding a new entity, can we not just use selectAllCmd.label from
> utilityOverlay.dtd?
> Plus that is what my local patch had in, just not had chance to do anything
> with it as I got distracted by build issues.
> Also has the advantage of being usable for aurora and beta.
Fixed.

Comment 7

[Ian Neal]

Comment on attachment 8466797 [details] [diff] [review]
Patch v2.0 Branch safe patch.

>+++ b/suite/mailnews/compose/messengercompose.xul

>+<!ENTITY % utilityDTD SYSTEM "chrome://communicator/locale/utilityOverlay.dtd">
>+%utilityDTD;

As this file is overlayed by utilityOverlay.xul you should not need to explicitly pull in utilityOverlay.dtd
r=me with that addressed.

Comment 8

[Philip Chee]

(In reply to Ian Neal from comment #7)

> >+<!ENTITY % utilityDTD SYSTEM "chrome://communicator/locale/utilityOverlay.dtd">
> >+%utilityDTD;

> As this file is overlayed by utilityOverlay.xul you should not need to
> explicitly pull in utilityOverlay.dtd
> r=me with that addressed.
Um Yes I *do* need to explicitly pull in utilityOverlay.dtd. DTD entities don't propagate.

Comment 9

[Ian Neal]

As per IRC conversation on Tuesday r=me with no changes

Comment 10

[Philip Chee]

Comment on attachment 8466797 [details] [diff] [review]
Patch v2.0 Branch safe patch.

[Approval Request Comment]
Regression caused by (bug #): 650885
User impact if declined: menu item missing - regression
Testing completed (on m-c, etc.): been baking in my tree since August.
Risk to taking this patch (and alternatives if risky): None - regression fix.
String changes made by this patch: NONE this is a branch safe patch.

Comment 11

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/a989882e01da

Pushed to all branches:
https://hg.mozilla.org/releases/comm-aurora/rev/cc23764d5227
https://hg.mozilla.org/releases/comm-beta/rev/d6186e633330

Pushed to comm-release (2.29.1)
https://hg.mozilla.org/releases/comm-release/rev/c9ebeab8ccf6