Rename 'Add' button to 'New' in Thunderbird Preferences, Display, Tags

RESOLVED FIXED in Thunderbird 53.0

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ovari123, Assigned: Paenglab)

Tracking

51 Branch
Thunderbird 53.0

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Posted image New Tag.png
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161119191213

Steps to reproduce:

Suggestions:

1. Renaming "_A_dd" button in attached image to "_N_ew" for consistency.
2. Rename "Create New Tab" to "New Tag"
3. keyboard shortcuts for "C_a_ncel" and "_O_K"

Thank you
If possible, can a global find and replace be done for "Add" to "_N_ew"?

Thank you
Hmm, for example on "Message Filters" we have "New".
Also the Address Book uses "New".

It's clear te tag one is not consistent with "Add" button and "Create New Tag" title. But we have also in Composition/General > Keywords dialog an "Add" button with consistent dialog title. Also in Attachments/Outgoing and Calendar/Categories we have "Add" buttons.

Technically both, "Add" and "New" are correct and we could be consistent at least in the Options/Preferences, which would mean only the "Create New Tag" title is wrong.

Globally it would be better to use "New". We are very late in TB 52 cycle for string changes. Jörg, what do you think about string changes at this stage?
Flags: needinfo?(jorgk)
(In reply to Richard Marti (:Paenglab) from comment #3)
> Globally it would be better to use "New". We are very late in TB 52 cycle
> for string changes. Jörg, what do you think about string changes at this
> stage?
I don't understand the string change policy either :-(

So far I haven't uplifted any string changes. I was asked to hold back until all late strings are ready and then uplift them in on go before Dec. 5th, 2016.

So there is no harm done in changing any strings now, which we may or may not uplift.
Flags: needinfo?(jorgk)
(In reply to Jorg K (GMT+1) from comment #4)

** 52 ESR: Discussion about string changes: Some TB and calendar bugs with string changes still to land/uplift. Should only announce to the L10N list only *once* when it's done. Will review during next meeting on Nov. 29th, 2016
https://public.etherpad-mozilla.org/p/thunderbird-status-meeting-minute-taking

I don't understand the string change policy either; however, based on quote above it seems like string changes can still make TB52.

Thank you
Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Posted patch changeAddToNew.patch (obsolete) — Splinter Review
Renamed all "Add" to "New..." in options/Preferences window.

I let now also open the tag dialog in the center of the options window.

MakeMyDay, I added you for the Calendar part. I hope we can this strings to TB 52/LTG 5.4.
Attachment #8814562 - Flags: review?(makemyday)
Attachment #8814562 - Flags: review?(jorgk)
Hmm, I must put my very picky hat on here.

"Add" and "New" are not really the same. "New" implies that the system creates something new for you, "Add" implies that you add something that already exists to a given set.

Let's see:
This patch has four parts:
1) Calendar: Create a "new" category. A category is an object that the systems
   creates and here you can create a new one. Good.
2) Tags: Same here, the tag is an object created by the system and you create a new one. Good.
3) Cloud account: Well, a cloud account in *not* anything the system creates, you get
   yourself such an account and then you *add* this account to the configuration.
   Personally, I'd leave "Add" here.
4) Reminder keywords: Well, a word is also nothing that the system creates.
   You *add* a word which already exists in your language to the set of words
   that TB will use to detect missing attachments. I'd leave "Add" here.

Too picky?

Thomas, do you have a (short!) opinion here? "Add" vs. "New"? We have for example "New" filter, but "Add" in other occasions. The spell checker for example has "Add Word", which makes sense.
Flags: needinfo?(bugzilla2007)
(In reply to Jorg K (GMT+1) from comment #7)
> Hmm, I must put my very picky hat on here.
> 
> "Add" and "New" are not really the same. "New" implies that the system
> creates something new for you, "Add" implies that you add something that
> already exists to a given set.
> 
> Let's see:
> This patch has four parts:
> 1) Calendar: Create a "new" category. A category is an object that the
> systems
>    creates and here you can create a new one. Good.
> 2) Tags: Same here, the tag is an object created by the system and you
> create a new one. Good.
> 3) Cloud account: Well, a cloud account in *not* anything the system
> creates, you get
>    yourself such an account and then you *add* this account to the
> configuration.
>    Personally, I'd leave "Add" here.
> 4) Reminder keywords: Well, a word is also nothing that the system creates.
>    You *add* a word which already exists in your language to the set of words
>    that TB will use to detect missing attachments. I'd leave "Add" here.

With 3) I'm okay. I was also not 100% sure to change it because without a cloud account we have the text "*Add* a new Filelink storage service".

With 4) I don't agree. This are also objects which are created by the system. Where are also predefined keywords like we have for the categories and tags.
(In reply to Richard Marti (:Paenglab) from comment #8)
> With 4) I don't agree. This are also objects which are created by the
> system. Where are also predefined keywords like we have for the categories
> and tags.
OK ;-)
So just remove 3).
Flags: needinfo?(bugzilla2007)
Not changed the cloud accounts from "Add" to "New" but to "Add...". ;-)
Attachment #8814562 - Attachment is obsolete: true
Attachment #8814562 - Flags: review?(makemyday)
Attachment #8814562 - Flags: review?(jorgk)
Attachment #8814606 - Flags: review?(makemyday)
Attachment #8814606 - Flags: review?(jorgk)
Comment on attachment 8814606 [details] [diff] [review]
changeAddToNew.patch v2

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

Yep. The patch looks good and I tried the four items mentioned in comment #7. r=jorgk
Attachment #8814606 - Flags: review?(jorgk) → review+
(In reply to Jorg K (GMT+1) from comment #7)
> Hmm, I must put my very picky hat on here.
> 
> "Add" and "New" are not really the same. "New" implies that the system
> creates something new for you, "Add" implies that you add something that
> already exists to a given set.
> [...]
> Too picky?

No...

> Thomas, do you have a (short!) opinion here? "Add" vs. "New"? We have for
> example "New" filter, but "Add" in other occasions. The spell checker for
> example has "Add Word", which makes sense.

My spontaneous reaction after reading Jörg's comment was "spot-on", exactly that!
Nothing to add or subtract.

I also feel that "Add" is slightly better than "New" for attachment reminder keywords, you add an existing word from the language to the list of attachment reminder keyword. While it could be argued that you'll be creating a new special object called attachment reminder keyword, it's still true that it's just a list of mostly natural words. So for dictionary type of things, "Add" sounds slightly more appropriate than "New".
Well, I was staring at the panel and "**New* reminder keyword" made some sense, like "new" list entry. So I can accept Richard's suggestion. Let's close the issue ;-)
Comment on attachment 8814606 [details] [diff] [review]
changeAddToNew.patch v2

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

I'm fine with the renaming, but not with using n as an accesskey - at least if you want to uplift this to esr52 that late in the cycle. <ALT> + n is currently the accesskey for the Events and Tasks in the main menu. Assigning accesskeys twice will end in none of them being functional.

If you would want to change Events and Tasks accesskey to be able to use n for your patch, this needs a full regression testing for all of the (calendar) accesskeys. I remember an accesskeys conflict resolution patch for the event dialog last year required a lot of changes at the end while initially only 2 or 3 strings were subject to change.

That said, we should be quite conservative to change accesskeys and limit this to conflict scenarios only. Also, this puts a lot of burden on keyboard centric user who have been using the product for quite a while already (which would be even worse if they depend on using screenreaders). And if we change accesskeys, this needs a thoroughly testing to avoid regressions.

You have also the option to use accesskeys which are not part of the label, so you could leave it with the original accesskeys (which would end in displaying <New... (A)>
Attachment #8814606 - Flags: review?(makemyday) → review-
(In reply to [:MakeMyDay] from comment #14)
> Comment on attachment 8814606 [details] [diff] [review]
> changeAddToNew.patch v2
> 
> Review of attachment 8814606 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm fine with the renaming, but not with using n as an accesskey - at least
> if you want to uplift this to esr52 that late in the cycle. <ALT> + n is
> currently the accesskey for the Events and Tasks in the main menu. Assigning
> accesskeys twice will end in none of them being functional.

Hmm, the accesskeys are in different windows. I don't see a conflict. If yes, we have already a conflict with E for Edit and the Edit main menu. But we are in different windows. Only the focused window get the accesskey. Also in In-Content prefs it does work as you have use ALT+SHIFT+A (or now N).
The conflict exists in In-Content mode. The problem is, that other then E, both n and N are used as accesskeys in the Events and Tasks menu (the former for opening the menu itself, the latter for the new event menu item). Maybe bug 1300845 will result in a change for the n case.

You can easily test using by trying to use the "new" accesskeys without your patch applied. They are only save, if no action is triggered for any of them (for bothh, with and without In-Content mode).
(In reply to [:MakeMyDay] from comment #14)
> ... displaying <New... (A)>
That's not crash hot. Maybe we take without uplift assuming the "n" issue is resolved?
MakeMyDay, can you explain to me what In-Content problem is, please. What is "In-Content"? When I have the modal options window on the screen <alt>N will go to this menu, it doesn't trigger the main menu's "Events and Task".
(In reply to [:MakeMyDay] from comment #16)
> The conflict exists in In-Content mode. The problem is, that other then E,
> both n and N are used as accesskeys in the Events and Tasks menu (the former
> for opening the menu itself, the latter for the new event menu item). Maybe
> bug 1300845 will result in a change for the n case.

Correction: N not n may change in context of bug 1300845.

(In reply to Jorg K (GMT+1) from comment #17)
> (In reply to [:MakeMyDay] from comment #14)
> > ... displaying <New... (A)>
> That's not crash hot. Maybe we take without uplift assuming the "n" issue is
> resolved?

Landing without uplift would be ok if done, however I would appreciate a full regression test as described in comment 16 also in that case, as I assume we don't have sufficient coverage by our mozmill tests for accesskeys. And a backlink in bug 1300845 would be nice if not already resolved.

But probably it's best to wait and see how bug 1300845 moves forward until our planned late-l10n checkins.

(In reply to Jorg K (GMT+1) from comment #18)
> MakeMyDay, can you explain to me what In-Content problem is, please. What is
> "In-Content"? When I have the modal options window on the screen <alt>N will
> go to this menu, it doesn't trigger the main menu's "Events and Task".

This is displaying the preferences in a tab instead of the modal dialog - set mail.preferences.inContent to true to enable it.
Ah okay the ALT+SHIFT+N. But then we have also the problem that this key is also used for _N_ew Message in Message menu.

But: When I type ALT+n, the Events and Tasks menu opens with the New Task item selected. With closed In-content prefs ALT+SHIFT+n opens again the Events and Tasks menu with the New Task item selected. On both cases I have to press enter to get the new dialog.

First the "New Event" accesskey is wrong when ALT+SHIFT+n is valid in global context as "New Message" owns it already. And ALT+SHIFT+n newer had the in bug 1300845intended function to select "New Event". It was always detected as ALT+n. You can check it by disabling Lightning and then press ALT+SHIFT+n. Nothing happens. So this concern and bug 1300845 aren't valid. ALT+SHIFT is used to access content keystrokes and not as global accesskey.
(In reply to [:MakeMyDay] from comment #19)
> This is displaying the preferences in a tab instead of the modal dialog -
> set mail.preferences.inContent to true to enable it.
I didn't know about that. This really makes my days since I've always been asking myself how TB maintained the modal window when FF has moved away from it a long time ago. It's amazing that this works. When did that get implemented?

I'll let you guys sort out the shortcuts. Looks like this won't ship in TB 52 ESR.
(In reply to Richard Marti (:Paenglab) from comment #20)
> Ah okay the ALT+SHIFT+N. But then we have also the problem that this key is
> also used for _N_ew Message in Message menu.
> 
> But: When I type ALT+n, the Events and Tasks menu opens with the New Task
> item selected. With closed In-content prefs ALT+SHIFT+n opens again the
> Events and Tasks menu with the New Task item selected. On both cases I have
> to press enter to get the new dialog.
> 
> First the "New Event" accesskey is wrong when ALT+SHIFT+n is valid in global
> context as "New Message" owns it already. And ALT+SHIFT+n newer had the in
> bug 1300845intended function to select "New Event". It was always detected
> as ALT+n. You can check it by disabling Lightning and then press
> ALT+SHIFT+n. Nothing happens. So this concern and bug 1300845 aren't valid.
> ALT+SHIFT is used to access content keystrokes and not as global accesskey.

Ok, I had a second look at this (although I cannot test the patch atm due to a broken build). There shouldn't be an issue wit using ALT+SHIFT+n as the shortcut should currently only be available in the context of an expanded Events and Tasks menu list, which in turn can be triggered by ALT+n. So r+ for landing on cc. However, before we decide whether to pick this for ESR 52, I would prefer to see this backing some days on trunk. This is still risky if it not thoroughly tested entirely for regressions.

(In reply to Jorg K (GMT+1) from comment #21)
> (In reply to [:MakeMyDay] from comment #19)
> > This is displaying the preferences in a tab instead of the modal dialog -
> > set mail.preferences.inContent to true to enable it.
> I didn't know about that. This really makes my days since I've always been
> asking myself how TB maintained the modal window when FF has moved away from
> it a long time ago. It's amazing that this works. When did that get
> implemented?

This exists since TB 38 - all credits are going to Richard (and Aceman iirc).
Attachment #8814606 - Flags: review- → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/1b384978d5f47dee6b8bf4325a9231c4b75c7ec4
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
Richard, do you want this uplifted?
Flags: needinfo?(richard.marti)
Comment on attachment 8814606 [details] [diff] [review]
changeAddToNew.patch v2

[Approval Request Comment]
User impact if declined: not 100% correct meaning of button strings
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): should be no risk of doubled accesskeys
Flags: needinfo?(richard.marti)
Attachment #8814606 - Flags: approval-comm-aurora?
Comment on attachment 8814606 [details] [diff] [review]
changeAddToNew.patch v2

Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/807f38e371b55e148759cee3fe10f7653cd2d89f
Attachment #8814606 - Flags: approval-comm-aurora? → approval-comm-aurora+
For the record, this mixes buttons with different classes of words on it: "New" (adjective) and "Edit" / "Delete" (verbs), e.g. in Options > Display > Tags.
You need to log in before you can comment on or make changes to this bug.