Closed Bug 1039933 Opened 10 years ago Closed 10 years ago

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

Categories

(SeaMonkey :: MailNews: Composition, defect)

SeaMonkey 2.26 Branch
x86_64
Windows 8.1
defect
Not set
normal

Tracking

(seamonkey2.29 fixed, seamonkey2.30 fixed, seamonkey2.31 fixed, seamonkey2.32 fixed)

RESOLVED FIXED
seamonkey2.32
Tracking Status
seamonkey2.29 --- fixed
seamonkey2.30 --- fixed
seamonkey2.31 --- fixed
seamonkey2.32 --- fixed

People

(Reporter: okohurgo, Assigned: philip.chee)

References

()

Details

(Keywords: regression)

Attachments

(3 files)

Attached image 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.
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.
Assignee: nobody → iann_bugzilla
Blocks: 650885
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(iann_bugzilla)
Keywords: regression
I do not see this on latest trunk on linux, there is no empty line.
Is this reproducible for latest trunk on other platforms?
Flags: needinfo?(iann_bugzilla) → needinfo?(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.
Flags: needinfo?(philip.chee) → needinfo?(iann_bugzilla)
Fix missing label for "Select All" attachment area context menu.
Assignee: iann_bugzilla → philip.chee
Status: NEW → ASSIGNED
Attachment #8466308 - Flags: review?(iann_bugzilla)
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.
Flags: needinfo?(iann_bugzilla) → needinfo?(philip.chee)
(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.
Attachment #8466797 - Flags: review?(iann_bugzilla)
Flags: needinfo?(philip.chee)
Attachment #8466308 - Flags: review?(iann_bugzilla)
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.
Attachment #8466797 - Flags: review?(iann_bugzilla) → review+
(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.
Flags: needinfo?(iann_bugzilla)
As per IRC conversation on Tuesday r=me with no changes
Flags: needinfo?(iann_bugzilla)
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.
Attachment #8466797 - Flags: approval-comm-release?
Attachment #8466797 - Flags: approval-comm-beta?
Attachment #8466797 - Flags: approval-comm-aurora?
Attachment #8466797 - Flags: approval-comm-release?
Attachment #8466797 - Flags: approval-comm-release+
Attachment #8466797 - Flags: approval-comm-beta?
Attachment #8466797 - Flags: approval-comm-beta+
Attachment #8466797 - Flags: approval-comm-aurora?
Attachment #8466797 - Flags: approval-comm-aurora+
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
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.32
You need to log in before you can comment on or make changes to this bug.