1391933 - Don't expose SUMO link as localizable string in Kill Addon dialog

Status: RESOLVED FIXED

(WebExtensions :: Frontend, enhancement)

Description

[Francesco Lodolo [:flod]]

https://hg.mozilla.org/mozilla-central/rev/6aa43c3fc0c5

+# LOCALIZATION NOTE (processHang.add-on.label): The first %S is the name of
+# an extension. The second %S is the name of the product (e.g., Firefox)
+processHang.add-on.label = A script in the extension “%S” is causing %S to slow down.

If there are multiple placeholders, use %1$S and %2%S.


+# LOCALIZATION NOTE (KillAddonScriptMessage): The first %S is the name of an add-on. The second %S is the name of the application (e.g., Firefox).
+KillAddonScriptMessage=A script from the add-on “%S” is running on this page, and making %S unresponsive.\n\nIt may be busy, or it may have stopped responsing permanently. You can stop the script now, or you can continue to see if it will complete.

Same here.

NOTE: typo in the string (responsing).

Why is "add-on" in DOM strings and "extension" in BROWSER?

+processHang.add-on.learn-more.url = https://support.mozilla.org/en-US/kb/warning-unresponsive-script?cache=no#w_other-causes

This should have raised red flags in review:
* Why are you exposing this as a localizable string? It shouldn't be localized, and to change the link you would need to update the string ID, and deal with string frozen branches. It should be in the code.
* You never put en-US in a link to SUMO, let the system do its own locale detection.

Comment 1

[Francesco Lodolo [:flod]]

Is there any chance to have a quick fix landed? I'd really like to avoid exposing that URL for localization.

Comment 2

[Kris Maglione [:kmag]]

(In reply to Francesco Lodolo [:flod] from comment #0)
> Why is "add-on" in DOM strings and "extension" in BROWSER?

Because UX didn't give me any updated copy for the non-e10s version, since users aren't expected to see it in practice after 57, and the original versions both said "add-on".

> +processHang.add-on.learn-more.url =
> https://support.mozilla.org/en-US/kb/warning-unresponsive-
> script?cache=no#w_other-causes
> 
> This should have raised red flags in review:

Should it? Is it documented somewhere?

There's no particular review process for localized strings aside from what we have in terms of automated tests. I used a localized string with an included locale code because this is what I've been asked to do for MDN URLs in the past. If we need to do something different for SUMO links, then it should be documented somewhere, and people writing front-end copy should have an obvious way to find it.

(In reply to Francesco Lodolo [:flod] from comment #1)
> Is there any chance to have a quick fix landed? I'd really like to avoid
> exposing that URL for localization.

I'll try to post a patch today, but I'm quite busy at the moment.

Comment 3

[Francesco Lodolo [:flod]]

(In reply to Kris Maglione [:kmag] from comment #2)
> Should it? Is it documented somewhere?

I'm not sure, but that's a good point.

> There's no particular review process for localized strings aside from what
> we have in terms of automated tests. I used a localized string with an
> included locale code because this is what I've been asked to do for MDN URLs
> in the past. If we need to do something different for SUMO links, then it
> should be documented somewhere, and people writing front-end copy should
> have an obvious way to find it.

I've seen MDN links break when doing locale redirection in the past, but I think that was fixed a long time ago.

Having said that, MDN links are usually put inside a sentence in console errors (you can filter by clicking DOM on top)
https://transvision.mozfr.org/?recherche=developer.mozilla.org&repo=central&sourcelocale=en-US&locale=en-US&search_type=strings

(side note: we should use en or en-US consistently…)

As you can note, there are no strings where the only content is a link. That happens a couple of time in DevTools, and should likely be fixed.

Comment 4

[Francesco Lodolo [:flod]]

Attachment: Bug 1391933 - Fix localization issues in Kill Add-on dialogs,

Changes originally landed in bug 1356334:
* Remove SUMO strings as a localizable string, remove en-US from link.
* Fix typo ("responsing").
* Use extensions instead of add-on in DOM strings for consistency.
* Remove period from KillAddonScriptGlobalMessage, since it's a checkbox.

Review commit: https://reviewboard.mozilla.org/r/170490/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170490/

Comment 5

[Francesco Lodolo [:flod]]

I hope I didn't break anything in the process…

Note that these changes would normally require a new string ID
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

But since I'm in charge of pushing strings from mozilla-central to the repository used by tools (and these strings haven't been exposed yet), I can skip that part.

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 8899228 [details]
Bug 1391933 - Fix localization issues in Kill Add-on dialogs,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170490/diff/1-2/

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8899228 [details]
Bug 1391933 - Fix localization issues in Kill Add-on dialogs,

https://reviewboard.mozilla.org/r/170490/#review176146

Thanks

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2486c6e1f89d
Fix localization issues in Kill Add-on dialogs, r=kmag

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2486c6e1f89d

Comment 10

[Stefan Plewako [:stef]]

I'm sorry but I need to reopen this. It is not OK and safe to reuse strings like below.

https://hg.mozilla.org/mozilla-central/diff/b83aea215a82/browser/locales/en-US/chrome/browser/browser.properties
> processHang.label = A web page is slowing down your browser. What would you like to do?
>+# LOCALIZATION NOTE (processHang.add-on.label): The first %S is the name of
>+# an extension. The second %S is the name of the product (e.g., Firefox)
>+processHang.add-on.label = A script in the extension "%S" is causing %S to slow down.
>+processHang.add-on.learn-more.text = Learn more
>+processHang.add-on.learn-more.url = https://support.mozilla.org/en-US/kb/warning-unresponsive-script?cache=no#w_other-causes
> processHang.button_stop.label = Stop It
> processHang.button_stop.accessKey = S
> processHang.button_wait.label = Wait
> processHang.button_wait.accessKey = W
> processHang.button_debug.label = Debug Script
> processHang.button_debug.accessKey = D

It may work for English but we need translate differently processHang.button_stop.label for addon/extension/web page/ ("Zatrzymaj ją" vs "Zatrzymaj go").

Comment 11

[Francesco Lodolo [:flod]]

I told you in the past: please file new bugs (yours definitely a legitimate request), don't reopen bugs assigned with landed patches.

Comment 12

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.