1193285 - Inconsistence in add-ons updating messages

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect, P5)

Description

[sworddragon2]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150807094550

Steps to reproduce:

1. Start Firefox and make sure you have an extension installed that is currently updateable.
2. Go to the menu bar -> Tools -> Add-ons -> Extensions.
3. Click on the dropdown field with the gear symbol on it and select "Check for Updates".
4. After the add-on has updated repeat step 3.


Actual results:

The message "Your add-ons have been updated." is ending with a "." while the messages "Updating add-ons" and "No updates found" aren't.


Expected results:

I think either no message or all of them should end with a ".".

Comment 1

[CosminB]

Following the steps from description of this bug I can reproduce this issue on Firefox 53.0a1(2017-01-16), Firefox 52.0a2(2017-01-16), Firefox 51.0b14(20170112171116), Firefox 50.1.0(20161208153507) under Wind 7 64-bit and Ubuntu 16.04 LTS 32-bit. 

Here is a video for this issue:https://www.screencast.com/t/taH0t08dPb77 

I am not sure if this issue is worth fixing, Andy what do you think about this?

Comment 2

[Andy McKay]

Its a minor inconsistency, let's see if anyone wants to pick it up. The line of code is here:

https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd#70

It's probably just a matter of removing that period. If this is your first time building Firefox, you'll want to get started here:

http://areweeveryoneyet.org/onramp/desktop.html

Comment 3

[Aman Mehta]

Hey I would like to take up this bug. Can you guide me on how do I submit my patch (local changes) to you so that you can review them.
Thanks

Comment 4

[Andy McKay]

Once you've got your patch, you can use MozReview to push the patch, there's a document that discusses this here:

http://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html

Your commit message for the patch should be something like: "bug 1193285 fix typo r?aswan"... since we'll technically need aswan to review this.

Comment 5

[Andrew Swan [:aswan]]

I believe that pushing to reviewboard requires level 1 commit access.  If you don't have that, you can use the bzexport extension to put the patch into an attachment here:
http://mozilla-version-control-tools.readthedocs.io/en/latest/hgext.html#bzexport

Assuming your changes are just to a strings file, please direct the review request to flod (i.e, put r?flod into the commit message).  There are a bunch of specific rules about updating internationalized strings and he is the expert on them.

Comment 6

[Aman Mehta]

Attachment: Please review this patch

Comment 7

[Andrew Swan [:aswan]]

Comment on attachment 8828062 [details] [diff] [review]
Please review this patch

You have a bunch of changes here to other messages that aren't part of this bug, plus what appears to be changes for bug 1253719.  Please strip the patch down to just the thing affected by this bug.  When your patch is ready for review, set the review flag to ? to indicate that you are requesting a review.

Comment 8

[Aman Mehta]

Attachment: Only add-on update messages have been corrected. If all other messages/labels are also to be corrected let me know.

Please review this patch.

Comment 9

[Andy McKay]

The patch still changes two other strings, "Your add-on updates have been downloaded", "Sorry, but there was an error loading the release notes". Is that intentional? Do those strings need changing? I'm not the context of where those message appear to the users.

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8828091 [details] [diff] [review]
Only add-on update messages have been corrected. If all other messages/labels are also to be corrected let me know.

This is close, thanks!

In addition to Andy's comments, please fix up the commit message: no quotes around the message, capitalize Bug, and move the r? bit to the end of the message.

When you upload a new patch, please mark this one obsolete (if you use hg bzexport, it should offer to do that for you automatically)

Comment 11

[Aman Mehta]

(In reply to Andy McKay [:andym] from comment #9)
> The patch still changes two other strings, "Your add-on updates have been
> downloaded", "Sorry, but there was an error loading the release notes". Is
> that intentional? Do those strings need changing? I'm not the context of
> where those message appear to the users.

Yes, it was made intentional so as to follow a standard of not having fullstops at the end of all add-on update messages. After add-on is updated, it shows "Your add-ons have been updated" whereas when add-on has been downloaded it shows "Your add-on updates have been downloaded."
Hence I removed the full-stops from other two messages which were related to add-on updates just to maintain a standard (of not ending with a fullstop for all add-on update messages).

Comment 12

[Aman Mehta]

Do you want me to keep those 2 extra messages to end with fullstop or should I remove them?

Comment 13

[Andrew Swan [:aswan]]

Scott, this bug is about whether messages in in-product dialogs include trailing periods or not.  What is the standard?

Comment 14

[sdevaney]

Please remove periods in this case, thank you.

Comment 15

[Aman Mehta]

Any problems with my last patch? This bug is completed right? If not, can you tell me what to do further and also assign this bug to me.

Thanks

Comment 16

[Andy McKay]

We need an r+ from aswan. I can see why "updates.downloaded.label" is used and that makes sense. I can't trigger "addon.errorLoadingReleaseNotes.label" but its down here: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.xml#842 so probably ok to change.

Comment 17

[Andrew Swan [:aswan]]

Francisco, are we okay to land these changes as-is?  (removing trailing periods to make messages consistent)
Aman, in any case, please update the commit message in your patch as mentioned in comment 10.

Comment 18

[Francesco Lodolo [:flod]]

Normally I would say update without a new string ID, the reality is that localizers don't always have the full picture, so they follow English in these cases.

Basically all languages ends with a period here
https://transvision.mozfr.org/string/?entity=toolkit/chrome/mozapps/extensions/extensions.dtd:updates.installed.label&repo=aurora

No periods here
https://transvision.mozfr.org/string/?entity=toolkit/chrome/mozapps/extensions/extensions.dtd:updates.updating.label&repo=aurora

If we want to ensure consistency across all languages, we need to make sure they double check their translations, and that's done by changing the string ID.
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Comment 19

[Andrew Swan [:aswan]]

Okay, Aman this got slightly more complicated, are you still interested in working on it?

Comment 20

[Aman Mehta]

Yes, sure.
Please guide me on how to proceed.

Comment 21

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.