989133 - Add a soft hyphen to the character encoding label for en-US to avoid auto-hyphenation

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Now that bug 962884 has landed, we can update some of the text in the en-US locale to use ­ hyphens. This will help buttons like "Character Encoding" from being displayed as "Character En-\r\n coding".

The meaning and semantics of the strings are not changing, so I'm not planning on changing any of the entity names. This should also be upliftable.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I placed all of the customizable widgets in the menu panel and I only saw poor auto hyphenation with the Character Encoding button.

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8398312 [details] [diff] [review]
Patch

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

Perhaps we can update the commit message and bug summary to indicate that only the character encoding widget changed?

Comment 3

[Francesco Lodolo [:flod]]

Have you guys tested this? From a quick test (unpacking brower/omni.jar, changing the string, repacking), the unicode character is just displayed in the menu :-\

Comment 4

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #3)
> Have you guys tested this? From a quick test (unpacking brower/omni.jar,
> changing the string, repacking), the unicode character is just displayed in
> the menu :-\

To clarify I didn't test with the updated code (which is supposed to remove the \u00ad in Australis menu), just with yesterday nightly.

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8398312 [details] [diff] [review]
Patch

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

::: browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
@@ +85,5 @@
>  # \u2026, or use "..." if \u2026 doesn't suit traditions in your locale.
>  feed-button.label = Subscribe
>  feed-button.tooltiptext = Subscribe to this page…
>  
> +# LOCALIZATOIN NOTE (characterencoding-button.label): The \u00ad character at the beginning

Just realized you misspelled LOCALIZATION

Comment 6

[Matthew N. [:MattN]]

(In reply to Francesco Lodolo [:flod] from comment #3)
> Have you guys tested this? From a quick test (unpacking brower/omni.jar,
> changing the string, repacking), the unicode character is just displayed in
> the menu :-\

:flod meant the escape sequence was appearing. I discussed this with him on IRC and he is posting an update to bug 962884 about what we learned. This is working fine for me so just comment 2 and comment 5 should be addressed still be Jared.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/983154994ca3

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/983154994ca3

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8398312 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Using new feature from bug 962884 to deal with the width of the new panel menu columns
User impact if declined: The Character Encoding widget label may be hyphenated instead of wrapped.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low risk adding a non-visible character to a string
String or IDL/UUID changes made by this patch: Yes, there is an addition of a soft hyphen character to characterencoding-button.label but it isn't visible and doesn't change the meaning (and therefore ID) of the string so I believe it doesn't break string freeze.

Comment 10

[Matthew N. [:MattN]]

Uplifting this feature also gives us manual test coverage of bug 962884 in en-US.

Comment 11

[Mike de Boer [:mikedeboer]]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/de453d0733ed

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8398312 [details] [diff] [review]
Patch

># HG changeset patch
># Parent 122bd8d437aa5792ff91041a65453bdc44db1b4b
># User Jared Wein <[e-mail]>
>Bug 989133 - Update en-US locale strings that have poor word-wrap characteristics to use &shy; character.
>
>diff --git a/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties b/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
>--- a/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
>+++ b/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
>@@ -81,17 +81,20 @@ paste-button.label = Paste
> # LOCALIZATION NOTE(paste-button.tooltiptext2): %S is the keyboard shortcut.
> paste-button.tooltiptext2 = Paste (%S)
> 
> # LOCALIZATION NOTE (feed-button.tooltiptext): Use the unicode ellipsis char,
> # \u2026, or use "..." if \u2026 doesn't suit traditions in your locale.
> feed-button.label = Subscribe
> feed-button.tooltiptext = Subscribe to this pageâ¦
> 
>-characterencoding-button.label = Character Encoding
>+# LOCALIZATOIN NOTE (characterencoding-button.label): The \u00ad character at the beginning
>+# of the string is used to disable auto hyphenation on the button text when it is displayed
>+# in the menu panel.
>+characterencoding-button.label = \u00adCharacter Encoding

I find this note rather unclear. It doesn't explain why we're disabling hyphenation, that this is very much en-US-specific and that localizers shouldn't blindly copy it. Maybe we should just have one note for all labels that explains where \u00ad should be used.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, I can see that being a problem. Would you like to make the change?

Comment 14

[Mike de Boer [:mikedeboer]]

Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/a9b5c0f4d674

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Is this covered by the automated test used for bug 962884?

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Florin Mezei, QA [:FlorinMezei] from comment #15)
> Is this covered by the automated test used for bug 962884?

Yeah it is.