Closed Bug 953659 Opened 10 years ago Closed 10 years ago

Fix various strings

Categories

(Instantbird Graveyard :: Localization, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: Mitch)

References

Details

Attachments

(1 file, 7 obsolete files)

*** Original post on bio 213 by Mitchell Field <mitch_1_2 AT live.com.au> at 2009-08-12 16:33:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch v1.0 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 185 by mitch_1_2 AT live.com.au at 2009-08-12 16:33:00 UTC ***

There are spelling and grammatical errors in various strings. This patch fixes
the errors that I noticed.
Assignee: nobody → bugzilla
Severity: normal → minor
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8351929 [details] [diff] [review]
Patch v1.0

*** Original change on bio 213 attmnt 185 at 2009-08-12 16:37:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351929 - Attachment is patch: true
Attachment #8351929 - Attachment mime type: application/octet-stream → text/plain
Attached patch Patch v1.1 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 187 by mitch_1_2 AT live.com.au at 2009-08-13 00:37:00 UTC ***

This patch also takes into account OS and localization for the "Help" menu.
Attachment #8351931 - Flags: review?(romain)
Comment on attachment 8351929 [details] [diff] [review]
Patch v1.0

*** Original change on bio 213 attmnt 185 by mitch_1_2 AT live.com.au at 2009-08-13 00:37:46 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351929 - Attachment is obsolete: true
Attached patch Patch v1.2 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 188 by mitch_1_2 AT live.com.au at 2009-08-13 00:53:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351932 - Flags: review?(romain)
Comment on attachment 8351931 [details] [diff] [review]
Patch v1.1

*** Original change on bio 213 attmnt 187 by mitch_1_2 AT live.com.au at 2009-08-13 00:53:28 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351931 - Attachment is obsolete: true
Attachment #8351931 - Flags: review?(romain)
Comment on attachment 8351932 [details] [diff] [review]
Patch v1.2

*** Original change on bio 213 attmnt 188 by mitch_1_2 AT live.com.au at 2009-08-13 00:55:04 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351932 - Flags: review?(florian)
Comment on attachment 8351931 [details] [diff] [review]
Patch v1.1

*** Original change on bio 213 attmnt 187 at 2009-08-13 01:00:04 UTC ***

> #ifdef XP_MACOSX
> <!-- Mac window menu -->
> #include ../../../../mozilla/toolkit/content/macWindowMenu.inc
> #endif
Please add these lines in the conditional branches bellow so that we have three distinct parts for each OS.

>-    <menu label="&help.menu;" id="aboutSeparator"> <!-- this id ensure it gets hidden on mac -->
>+#ifdef XP_WIN
>+    <menu label="&helpWin.menu;" id="helpMenu">
>+#elif defined(XP_MACOSX)
>+    <menu label="&help.menu;" id="aboutSeparator"><!-- This ID ensures it gets hidden. -->
>+#else
>+    <menu label="&help.menu;" id="helpMenu">
>+#endif

Looks good otherwise,
Even if you have my approval for this patch


>     for each (smile in gTheme.json.smileys) {
>-      for each (text in smile.texts)
>+      for each (text in smile.text)
>         gTheme.iconsHash[text] = smile;
>     }
Here is a point of incompatibility, but it is worth it while we don't have many Emoticon themes out.


>-<!ENTITY proxiesDialog.useEnvironemental     "Use Environemental Settings">
>-<!ENTITY proxiesDialog.noProxyInEnvironment  "No proxy settings found in the environment">
>+<!ENTITY proxiesDialog.useEnvironmental      "Use Environmental Settings">
>+<!ENTITY proxiesDialog.noProxyInEnvironment  "No proxy settings found">
"in the environment" was not necessary here because of the string ahead that is already explicit : "Use Environmental Settings"
Attachment #8351931 - Flags: review-
Comment on attachment 8351932 [details] [diff] [review]
Patch v1.2

*** Original change on bio 213 attmnt 188 at 2009-08-13 01:43:43 UTC ***

>+#elif defined(XP_WIN)
This line cause a parse error, I suggest the whole following structure (tested):

#ifdef XP_MACOSX
<!-- Mac window menu -->
#include ../../../../mozilla/toolkit/content/macWindowMenu.inc
    <menu label="&help.menu;" id="aboutSeparator"><!-- This ID ensures it gets hidden. -->
#else
#ifdef XP_WIN
    <menu label="&helpWin.menu;" id="helpMenu">
#else
    <menu label="&help.menu;" id="helpMenu">
#endif
#endif

Another solution was to put in common <menu and have two conditions, but it sounds harder to read so I think it does not worth it.
r+ with that fixed.
Attachment #8351932 - Flags: review?(romain) → review-
Attached patch Patch v1.3 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 189 by mitch_1_2 AT live.com.au at 2009-08-13 01:51:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351933 - Flags: review?(romain)
Comment on attachment 8351932 [details] [diff] [review]
Patch v1.2

*** Original change on bio 213 attmnt 188 by mitch_1_2 AT live.com.au at 2009-08-13 01:51:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351932 - Attachment is obsolete: true
Attachment #8351932 - Flags: review?(florian)
Comment on attachment 8351933 [details] [diff] [review]
Patch v1.3

*** Original change on bio 213 attmnt 189 at 2009-08-13 08:40:38 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351933 - Flags: review?(romain) → review+
Attached patch Patch v1.4 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 190 by mitch_1_2 AT live.com.au at 2009-08-13 08:55:00 UTC ***

Sorry, the previous patch was incomplete/broken.
Attachment #8351934 - Flags: review?(romain)
Comment on attachment 8351933 [details] [diff] [review]
Patch v1.3

*** Original change on bio 213 attmnt 189 by mitch_1_2 AT live.com.au at 2009-08-13 08:55:38 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351933 - Attachment is obsolete: true
Comment on attachment 8351934 [details] [diff] [review]
Patch v1.4

*** Original change on bio 213 attmnt 190 by mitch_1_2 AT live.com.au at 2009-08-13 08:57:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351934 - Flags: review?(florian)
Attached patch Patch v1.5 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 191 by mitch_1_2 AT live.com.au at 2009-08-13 09:31:00 UTC ***

Typo fix. Blargh.
Attachment #8351935 - Flags: review?(romain)
Comment on attachment 8351934 [details] [diff] [review]
Patch v1.4

*** Original change on bio 213 attmnt 190 by mitch_1_2 AT live.com.au at 2009-08-13 09:31:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351934 - Attachment is obsolete: true
Attachment #8351934 - Flags: review?(romain)
Attachment #8351934 - Flags: review?(florian)
Comment on attachment 8351935 [details] [diff] [review]
Patch v1.5

*** Original change on bio 213 attmnt 191 by mitch_1_2 AT live.com.au at 2009-08-13 09:33:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351935 - Flags: review?(florian)
Comment on attachment 8351935 [details] [diff] [review]
Patch v1.5

*** Original change on bio 213 attmnt 191 at 2009-08-13 09:38:07 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351935 - Flags: review?(romain) → review+
Comment on attachment 8351935 [details] [diff] [review]
Patch v1.5

*** Original change on bio 213 attmnt 191 at 2009-08-24 22:00:32 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351935 - Flags: review?(florian) → review-
*** Original post on bio 213 at 2009-08-24 22:00:33 UTC ***

Comment on attachment 8351935 [details] [diff] [review] (bio-attmnt 191)
Patch v1.5

>diff --git a/instantbird/base/content/instantbird/account.js b/instantbird/base/content/instantbird/account.js

>-      result = bundle.getString("proxies.useEnvironemental");
>+      result = bundle.getString("proxies.useEnvironmental");

"useEnvironmental" looks meaningless to me. What about "useEnvironment"?

>diff --git a/instantbird/base/content/instantbird/imSmileys.jsm b/instantbird/base/content/instantbird/imSmileys.jsm
>--- a/instantbird/base/content/instantbird/imSmileys.jsm
>+++ b/instantbird/base/content/instantbird/imSmileys.jsm
>@@ -82,17 +82,17 @@ function getSmileyList(aTheme) {
>     aTheme = getTheme();
> 
>   if (!aTheme.json)
>     return null;
> 
>   let addAbsoluteUrls = function(aSmiley) {
>     return {filename: aSmiley.filename,
>             src: aTheme.baseUri + aSmiley.filename,
>-            texts: aSmiley.texts};
>+            text: aSmiley.text};

I understand that "texts" is wrong, but the idea was "list of text representations", so "text" without the "s" doesn't convey the desired meaning.

We need something short here (it will be repeated over and over again in smiley theme files).
After reading this sentence "Many applications use text codes, which become replaced with a graphical emoticon." in the wikipedia article for "Emoticon", I suggest we use "textCodes".

>diff --git a/instantbird/base/content/instantbird/smiley.css b/instantbird/base/content/instantbird/smiley.css
>--- a/instantbird/base/content/instantbird/smiley.css
>+++ b/instantbird/base/content/instantbird/smiley.css
>@@ -48,15 +48,15 @@ listheader {
> treecol {
>   -moz-binding: url("chrome://instantbird/content/smiley.xml#treecol");
> }
> 
> smiley:nth-child(odd) {
>   background-color: -moz-oddtreerow;
> }
> 
>-.smileyTexts {
>+.smileyText {
>   font-family: monospace;
> }
I don't mind using smileyText here, even if it lose the sense of the list. After all, this is only a CSS class...

>diff --git a/instantbird/base/content/instantbird/smileys.js b/instantbird/base/content/instantbird/smileys.js
>--- a/instantbird/base/content/instantbird/smileys.js
>+++ b/instantbird/base/content/instantbird/smileys.js
>@@ -85,16 +85,16 @@ var smileysPreview = {
>       list.removeChild(item);
>       item = next;
>     }
> 
>     if (this.smileyList) {
>       for each (let smiley in this.smileyList) {
>         let item = document.createElement("smiley");
>         item.setAttribute("smileyImage", smiley.src);
>-        item.setAttribute("smileyTexts", smiley.texts.join(" "));
>+        item.setAttribute("smileyText", smiley.text.join(" "));

Hmm... Not sure if it's better to use smileyText, smileyCodes or smileyTextCodes here...

>diff --git a/instantbird/base/content/instantbird/smileys.xul b/instantbird/base/content/instantbird/smileys.xul
>--- a/instantbird/base/content/instantbird/smileys.xul
>+++ b/instantbird/base/content/instantbird/smileys.xul
>@@ -74,12 +74,12 @@
>       </menupopup>
>     </menulist>
>   </hbox>
>   <separator class="thin"/>
>   <description>&previewDescription;</description>
>   <richlistbox flex="1" id="preview">
>     <listheader equalsize="always">
>       <treecol id="imageColumn" label="&imageColumn.label;" flex="1"/>
>-      <treecol id="textsColumn" label="&textsColumn.label;" flex="1"/>
>+      <treecol id="textColumn" label="&textColumn.label;" flex="1"/>

"textRepresentationsColumn" maybe? That's a bit long...

>diff --git a/instantbird/locales/en-US/chrome/instantbird/proxies.dtd b/instantbird/locales/en-US/chrome/instantbird/proxies.dtd

>-<!ENTITY proxiesDialog.useEnvironemental     "Use Environemental Settings">
>-<!ENTITY proxiesDialog.noProxyInEnvironment  "No proxy settings found in the environment">
>+<!ENTITY proxiesDialog.useEnvironmental      "Use Environmental Settings">
>+<!ENTITY proxiesDialog.noProxyInEnvironment  "No proxy settings found">

I don't like the change of the noProxyInEnvironment string. "the environment" is much more likely to produce interesting results in a search engine than "environmental". I don't think the redundancy in this place is something worth caring about. Anyway, I'll be really happy when this whole dialog will be remove/replaced by a simpler way of configuring proxy settings ;).

>diff --git a/instantbird/locales/en-US/chrome/instantbird/smileys.dtd b/instantbird/locales/en-US/chrome/instantbird/smileys.dtd
>--- a/instantbird/locales/en-US/chrome/instantbird/smileys.dtd
>+++ b/instantbird/locales/en-US/chrome/instantbird/smileys.dtd
>@@ -1,7 +1,7 @@
> <!ENTITY smileysWindow.title "Smileys - &brandShortName;">
>-<!ENTITY themeLabel          "Emoticons Theme:">
>+<!ENTITY themeLabel          "Emoticon Theme:">
What's the rationale for this change?
Googlefighting "emoticons theme" vs "emoticon theme" doesn't give a clear winner.

>-<!ENTITY textsColumn.label   "Texts">
>+<!ENTITY textColumn.label    "Text">
I think we should put "Text Codes" here.

The other changes look good! Thanks for fixing my spelling mistakes!
Attached patch Patch v1.6 (obsolete) — Splinter Review
*** Original post on bio 213 as attmnt 220 by mitch_1_2 AT live.com.au at 2009-08-26 15:53:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351964 - Flags: review?(romain)
Comment on attachment 8351935 [details] [diff] [review]
Patch v1.5

*** Original change on bio 213 attmnt 191 by mitch_1_2 AT live.com.au at 2009-08-26 15:53:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351935 - Attachment is obsolete: true
Comment on attachment 8351964 [details] [diff] [review]
Patch v1.6

*** Original change on bio 213 attmnt 220 at 2009-08-26 16:43:56 UTC ***

>+    <menu label="&help.menu;" id="aboutSeparator" accesskey="&help.accesskey;><!-- This ID ensures it gets hidden. -->
A double quote is missing between ; and > here (it was ok on the previous version).


>-    for each (smile in gTheme.json.smileys) {
>-      for each (text in smile.texts)
>-        gTheme.iconsHash[text] = smile;
>+    for each (smiley in gTheme.json.smileys) {
>+      for each (textCodes in smiley.textCodes)
Please put "textCodes" in its singular form here, we had (text in smile.texts), we should have (textCode in smiley.textCodes).


>+<!ENTITY proxiesDialog.noProxyInEnvironment  "No proxy settings found in environment">
Isn't it "No proxy settings found in the environment"?

If flo has nothing to add (apparently not, at least for the moment :p), the next patch is the good one.
Attachment #8351964 - Flags: review?(romain) → review-
Attached patch Patch v1.7Splinter Review
*** Original post on bio 213 as attmnt 221 by mitch_1_2 AT live.com.au at 2009-08-26 22:25:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351965 - Flags: review?(romain)
Comment on attachment 8351964 [details] [diff] [review]
Patch v1.6

*** Original change on bio 213 attmnt 220 by mitch_1_2 AT live.com.au at 2009-08-26 22:25:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351964 - Attachment is obsolete: true
Comment on attachment 8351965 [details] [diff] [review]
Patch v1.7

*** Original change on bio 213 attmnt 221 by mitch_1_2 AT live.com.au at 2009-08-26 22:26:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351965 - Flags: review?(florian)
Comment on attachment 8351965 [details] [diff] [review]
Patch v1.7

*** Original change on bio 213 attmnt 221 at 2009-08-26 22:28:56 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351965 - Flags: review?(romain) → review+
Comment on attachment 8351965 [details] [diff] [review]
Patch v1.7

*** Original change on bio 213 attmnt 221 at 2009-08-26 22:57:12 UTC ***

Looks good, finally :).
Attachment #8351965 - Flags: review?(florian) → review+
*** Original post on bio 213 at 2009-08-26 23:07:30 UTC ***

Pushed as https://hg.instantbird.org/instantbird/rev/e310435921e9
Thanks!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.2b1
Depends on: 953673
Component: Other → Localization
*** Reassigning to the proper email address due to an incomplete mapping of
addresses during the BIO -> BMO merge. ***
Assignee: bugzilla → mitchell.field
You need to log in before you can comment on or make changes to this bug.