Thunderbird always displays "Delete" in Address Book edit menu, despite code being there for more (also port bug 112959)

RESOLVED FIXED in Thunderbird 3

Status

Thunderbird
Address Book
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 325811 [details] [diff] [review]
The fix

This started out as a port of bug 112959 (Select a list and the Edit menu either displays Delete Card or Delete Address Book), I then soon realised that TB's edit menu always displayed delete, and you can attempt to delete cards from read-only address books via the context menu.

The patch I'm attaching does the following:

- Port bug 112959 which means:
-- TB will display "Delete Card"/"Delete Selected Cards"/"Delete Address Book"/"Delete List"/"Delete Selected Lists"/"Delete Selected Items"
- Fix the Edit menu so that it is updated when the selection changes
- Fix the Results pane context menu "Delete" option, so that it is updated correctly for the selection.

There were two reasons for porting bug 112959; firstly it gives the user much better contextual information about what they are deleting when they use the Edit menu, secondly it syncs some TB code with SM code which will make the differences between the two less (which will help with some merging I'm doing).
Attachment #325811 - Flags: review?(mkmelin+mozilla)

Comment 1

10 years ago
Comment on attachment 325811 [details] [diff] [review]
The fix

Code wise this looks fine, but I'm not very convinced I like it. It's more informative sure, but the menu items get very long. 

And why the "selected" in "Delete Selected Cards", vs "Delete Card"? Then again, do people really think if the contacts as "cards"? 

Maybe Bryan can give some input on this...
Attachment #325811 - Flags: ui-review?(clarkbw)
(Assignee)

Comment 2

10 years ago
(In reply to comment #1)
> (From update of attachment 325811 [details] [diff] [review])
> Code wise this looks fine, but I'm not very convinced I like it. It's more
> informative sure, but the menu items get very long. 
> 
> And why the "selected" in "Delete Selected Cards", vs "Delete Card"? Then
> again, do people really think if the contacts as "cards"? 

Thunderbird consistently references all entries in the address book as "Cards", therefore I think we should keep that (at least for this bug).

wrt selected, I can see the point. I think we had this on SeaMonkey as it was already there, I agree we should see what Bryan thinks.
(Assignee)

Comment 3

10 years ago
btw, I also want to do something similar on the print/print preview menu options in the address book as they are just awful at the moment.
Comment on attachment 325811 [details] [diff] [review]
The fix

I have to agree, the selected seems odd.  Assuming the menu item is sensitive only when something is selected then it's a bit redundant.  

And the Card text would have to be another bug.  I'm a fan of Contact much more, but that's for another time.

Otherwise this looks great.
Attachment #325811 - Flags: ui-review?(clarkbw) → ui-review+
(Assignee)

Comment 5

10 years ago
(In reply to comment #4)
> (From update of attachment 325811 [details] [diff] [review])
> I have to agree, the selected seems odd.  Assuming the menu item is sensitive
> only when something is selected then it's a bit redundant.  

Ok, I'll drop the selected.
(Assignee)

Comment 6

10 years ago
Created attachment 327082 [details] [diff] [review]
The fix v2

I've dropped the "selected". As I changed the context of an existing string, I added "1" to its name so that l10n would pick it up as a change.
Attachment #325811 - Attachment is obsolete: true
Attachment #327082 - Flags: review?(mkmelin+mozilla)
Attachment #325811 - Flags: review?(mkmelin+mozilla)

Comment 7

10 years ago
Comment on attachment 327082 [details] [diff] [review]
The fix v2

Yes, I think this is much better. 

Nit: Align the value in 
+<!ENTITY deleteCards1Cmd.label                           "Delete Cards">

I'd also prefer something like deleteCardsCmd2.label as 1 is so easily confused with l.

r=mkmelin with that
Attachment #327082 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 8

10 years ago
Checked in with entity renamed as suggested -> fixed.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3
You need to log in before you can comment on or make changes to this bug.