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

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Mark Banner (:standard8)]

Attachment: 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).

Comment 1

[Magnus Melin [:mkmelin]]

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...

Comment 2

[Mark Banner (:standard8)]

(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.

Comment 3

[Mark Banner (:standard8)]

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 4

[Bryan Clark (DevTools PM) [:clarkbw]]

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.

Comment 5

[Mark Banner (:standard8)]

(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.

Comment 6

[Mark Banner (:standard8)]

Attachment: 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.

Comment 7

[Magnus Melin [:mkmelin]]

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

Comment 8

[Mark Banner (:standard8)]

Checked in with entity renamed as suggested -> fixed.