802046 - Typos, outdated infos and irregular extra whitespace in localization/l10n comments and strings

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

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

filterEditor.dtd
<!-- LOCALIZATION NOTE
The values below are used to control the widths of the filter action widgets.
Change the values only when the localized strings in the popup menus
are truncated in the widgets.
-->

Please add a link to the flex attribute on developer.mozilla.org

messenger.dtd
## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
## #1 is the count of items in the list.# filterCountItems is a pluralForm - see
filterCountItems=#1 Filter; #1 Filter

Remove " - see".


messengerCompose/sendProgress.dtd:

<!--LOCALIZATION NOTE sendprogress.dtd Main UI for Send Message Progress Dialog -->
<!ENTITY sendDialog.title "Processing Message">

sendprogress.dtd has been removed.

There are also sometimes than one subsequent whitespaces in l10n notes or strings, just look into messenger.dtd

Comment 1

[Ludovic Hirlimann [:Usul]]

Aryx can you come up with a patch ?

Comment 2

[:aceman]

I could do the patch too if Aryx has better things to do. Thanks for noticing the bugs.

Comment 3

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

Feel free to fix this patch, you could also leave the bug open for a new contributor (with [good first bug] in the whiteboard).

Comment 4

[:aceman]

Oooh, did I reach the level that I also have better things to do? :)

Ok, I'll see if I have something to do in my queue that is not blocked by review.

Comment 5

[kunal bansal]

I want to try on this as my first bug.but since i am still a novice can you suggest something?

Comment 6

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

You can read how to build Thunderbird at https://developer.mozilla.org/en-US/docs/Simple_Thunderbird_build
It's also recommended to use Mercurial's (hg) Mercurial Queues extension to easily keep an unchanged source tree: https://developer.mozilla.org/en-US/docs/Mercurial_Queues
You can find the source search at http://mxr.mozilla.org/comm-central/

Comment 7

[Thomas D. (:thomas8)]

(In reply to Archaeopteryx [:aryx] from comment #6)
> You can read how to build Thunderbird at
> https://developer.mozilla.org/en-US/docs/Simple_Thunderbird_build
> It's also recommended to use Mercurial's (hg) Mercurial Queues extension to
> easily keep an unchanged source tree:
> https://developer.mozilla.org/en-US/docs/Mercurial_Queues

fwiw, in this particular case I really don't see the point of exposing new contributors to the scary and tedious hassle of building TB while this is about nitfixes in a language resource file that are completely inconsequential to the build if done right. A simple file comparison utility that can produce patches semi-automatically in the right format will do. So I agree with aceman's whiteboard entry: [no compilation/build needed].

> You can find the source search at http://mxr.mozilla.org/comm-central/

Yep, so the current messenger.dtd source file can be found there using the search.

- download original source file (a)
- make changes as described above using text editor
- save changes into new file (b)
- use file comparison utility to produce diff file between (a) and (b), which needs to have correct patch format (look at simple patches from other bugs)

Comment 8

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. from comment #7)
Sorry but i have to disagree since not using hg is a dead end. Without it you can't really do anything except the very most trivial things, and it really is the easiest way to do get the source and produce patches anyway. (You can consider queues later though.)

You install mercurial, then
 - hg clone http://hg.mozilla.org/comm-central 
 - make the change
 - hg diff > bugxxx_fix.patch

... and you're done with he basics.

Comment 9

[kunal bansal]

"""
filterEditor.dtd
<!-- LOCALIZATION NOTE
The values below are used to control the widths of the filter action widgets.
Change the values only when the localized strings in the popup menus
are truncated in the widgets.
-->

Please add a link to the flex attribute on developer.mozilla.org
""""""
DONE--

""""""
messenger.dtd
## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
## #1 is the count of items in the list.# filterCountItems is a pluralForm - see
filterCountItems=#1 Filter; #1 Filter

Remove " - see".
"""""""
Cant find this statement in messenger.dtd


""""""
messengerCompose/sendProgress.dtd:

<!--LOCALIZATION NOTE sendprogress.dtd Main UI for Send Message Progress Dialog -->
<!ENTITY sendDialog.title "Processing Message">

sendprogress.dtd has been removed.

"""""""
i checked this statement no longer exist in messengercompose.std

""""""""
There are also sometimes than one subsequent whitespaces in l10n notes or strings, just look into messenger.dtd
""""""""
DONE--

Comment 10

[Ludovic Hirlimann [:Usul]]

Kunal can you also provide a patch and attach it on the bug ?

Comment 11

[kunal bansal]

Attachment: Patch

As described my last comment the changes have been made

Comment 12

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

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

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

This looks good except for one issue. Thank you for the work.

::: suite/locales/en-US/chrome/mailnews/FilterEditor.dtd
@@ +56,4 @@
>    Change the values only when the localized strings in the popup menus
>    are truncated in the widgets.
>   -->
> +<!-- Flex Attribute: https://developer.mozilla.org/en-US/docs/XUL/Attribute/flex

The comment doesn't get closed here, please fix it.

Comment 13

[kunal bansal]

Attachment: new patch

Comment 14

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

Comment on attachment 728604 [details] [diff] [review]
new patch

Thank you for fixing the patch and the patience.

Mike, can you review this? It touches only localization files. Thank you.

Comment 15

[kunal bansal]

my pleasure aryx,
Just one question,does it make any difference if the bug is assigned to me or not?

Comment 16

[Magnus Melin [:mkmelin]]

Not a lot, but it's good for bookkeeping :)

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 728604 [details] [diff] [review]
new patch

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

In general you shouldn't touch lines that don't need changing. messenger.dtd is really unchanged, just a lot of whitespace changes right?

::: mail/locales/en-US/chrome/messenger/FilterEditor.dtd
@@ +57,5 @@
>    Change the values only when the localized strings in the popup menus
>    are truncated in the widgets.
>   -->
> +<!-- Flex Attribute: https://developer.mozilla.org/en-US/docs/XUL/Attribute/flex
> +-->

Looks like this would fit nicely on one row:
<!-- Flex Attribute: https://developer.mozilla.org/en-US/docs/XUL/Attribute/flex -->

Comment 18

[kunal bansal]

ok thankyou i will remember this.
but i was just following the indentation of the previous comment actually i just lost the changes i made alng with lot of data on my disk.So can i again make all changes from the start again?

Comment 19

[kunal bansal]

can the changes also be made in the patch file directly?

Comment 20

[Magnus Melin [:mkmelin]]

No you have to generate a new patch, what's in bugzilla can't be edited.

Comment 21

[:aceman]

You can of course download it from here, edit and upload as a new attachment.

Comment 22

[kunal bansal]

Attachment: patch

melin please have a look at this

Comment 23

[Magnus Melin [:mkmelin]]

Comment on attachment 732154 [details] [diff] [review]
patch

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

For future reference, avoid making unrelated whitespace changes at least if it's not near what you're fixing.
Anyway, i guess that dtd spacing is really ugly, so r=mkmelin

Comment 24

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #17)
> In general you shouldn't touch lines that don't need changing. messenger.dtd
> is really unchanged, just a lot of whitespace changes right?

(In reply to Magnus Melin from comment #23)
> Review of attachment 732154 [details] [diff] [review]:
> -----------------------------------------------------------------
> For future reference, avoid making unrelated whitespace changes at least if
> it's not near what you're fixing.
> Anyway, i guess that dtd spacing is really ugly, so r=mkmelin

Probably these whitespace changes were done because they have been requested in this bug's comment 0:

(In reply to Archaeopteryx [:aryx] from comment #0)
> There are also sometimes than one subsequent whitespaces in l10n notes or
> strings, just look into messenger.dtd

In that sense, they do seem related to this bug ;)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

applying 802046
patching file mail/locales/en-US/chrome/messenger/FilterEditor.dtd
bad hunk #1 @@ -57,5 +57,7 @@
 (6 5 7 7)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 802046

While you're at it, can you please make sure your patch has the needed commit information in it? Thanks!
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 26

[kunal bansal]

Attachment: final patch

Few more changes have been made so it needs to be reviewed again.

Comment 27

[Magnus Melin [:mkmelin]]

Comment on attachment 746075 [details] [diff] [review]
final patch

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

::: mail/locales/en-US/chrome/messenger/messenger.dtd
@@ +1,3 @@
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
> + - License, v. 2.0. If a copy of the MPL was not distributed with this
> + - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->

This is a standard boilerplate, and should definitely NOT be reformatted.

@@ +814,3 @@
>  <!ENTITY search.keyLabel.nonmac "&lt;Ctrl+K&gt;">
>  <!-- LOCALIZATION NOTE (search.keyLabel.mac):
> + The description of the key-binding to get into the global search box on mac

I think the original is the most correct/preferred here. But please don't change the formatting of this and the other locailzation notes at all - there's simply no need for it.

Comment 28

[kunal bansal]

I Believe the necessary changes have already been made in the code and this bug is obsolete now

Comment 29

[kunal bansal]

I have verified all the changes have already been made into the comm-central.So no need of this bug.

Comment 30

[:aceman]

How is that possible?

Comment 31

[kunal bansal]

I am really sorry for my mistake i had erroneously deleted some patch files and wasnt able to revert changes.So i apologise 
I will update the patch soon

Comment 32

[kunal bansal]

Attachment: Patch

This is a better patch.Needs review.

Comment 33

[Thomas D. (:thomas8)]

I'm a bit worried about the lack of direction in this bug, which must be somewhat confusing and unpleasant for a new contributor like kunal bansal.

Therefore, I suggest that we be a lot more specific about what exactly we want or do not want changed here before going any further.

(In reply to Archaeopteryx [:aryx] from comment #0)

1) ***** filterEditor.dtd *****

> filterEditor.dtd
> <!-- LOCALIZATION NOTE
> The values below are used to control the widths of the filter action widgets.
> Change the values only when the localized strings in the popup menus
> are truncated in the widgets.
> -->
> 
> Please add a link to the flex attribute on developer.mozilla.org

I think this one is understood and addressed in the patch.
http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/FilterEditor.dtd#55

2) ***** filter.properties *****

> messenger.dtd
> ## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
> ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> ## #1 is the count of items in the list.# filterCountItems is a pluralForm -
> see
> filterCountItems=#1 Filter; #1 Filter
> 
> Remove " - see".

This is found in filter.properties, not messenger.dtd:
http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/filter.properties#43

I think we need to improve this more than just removing " - see".
I propose this as a minimal fix replacement:

43 ## LOCALIZATION NOTE(filterCountItems): Semicolon-separated list of singular and plural forms.
44 ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
45 ## #1 is the count of items in the list.
46 filterCountItems=#1 item; #1 items

Magnus, pls confirm if that's ok.

3) ***** sendProgress.dtd *****

> messengerCompose/sendProgress.dtd:
> 
> <!--LOCALIZATION NOTE sendprogress.dtd Main UI for Send Message Progress
> Dialog -->
> <!ENTITY sendDialog.title "Processing Message">
> 
> sendprogress.dtd has been removed.

:aryx, can you explain what exactly you wanted to change here? I don't understand. Both sendprogress.xul and sendprogress.dtd still exist.

http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/messengercompose/sendProgress.dtd

http://mxr.mozilla.org/comm-central/source/mailnews/compose/content/sendProgress.xul

4) ***** messenger.dtd *****

> There are also sometimes more than one subsequent whitespaces in l10n notes or
> strings, just look into messenger.dtd

This needs clearer instructions which whitespaces we want removed or not.

4a) Licence

http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/messenger.dtd#1

1 <!-- This Source Code Form is subject to the terms of the Mozilla Public
2    - License, v. 2.0. If a copy of the MPL was not distributed with this
3    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->

The above should remain unchanged, as stated by Magnus.

4b) Indentation of localization notes

I suggest the following indentation for localization comments (which seems to be the current default format):

19 <!-- LOCALIZATION NOTE (moveToNewWindow.label):
20      Menu option to cause the current tab to be migrated to a new Thunderbird
21      window.
22      -->

If we can agree that this is the desired indentation format, we should ask kunal to change it for all localization notes in that file, otherwise none. Magnus?

4c) Double spaces after full stops in sentences (localization notes)

http://blogs.telegraph.co.uk/news/damianthompson/100072634/why-using-two-spaces-after-a-full-stop-is-wrong-period/

I think double spaces after full stops are no longer recommended.
So they should be removed whereever they are, as kunal did in his last patch.

4d) More than one whitespace in <!ENTITY name "value">

https://developer.mozilla.org/en-US/docs/XUL/Tutorial/Localization

We currently have two competing formats (and several entries with a random number of spaces between name and value)

*** Format 1:

<!ENTITY name "somevalue">
<!ENTITY longername "someothervalue">

Single-spacing without visual alignment: This is easy to maintain, but very bad readability when it comes to large set of entities.

*** Format 2:
<!ENTITY name         "somevalue">
<!ENTITY longername   "someothervalue">

Space-filling for neat vertical alignment of values (localization strings):
I think this makes for much better human readability. For a nice example, see this: http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/messenger.dtd#457

457 <!ENTITY imStatus.available          "Available">
458 <!ENTITY imStatus.unavailable        "Unavailable">
459 <!ENTITY imStatus.offline            "Offline">
460 <!ENTITY imStatus.showAccounts       "Show Accounts…">

Notice how much easier you can read these compared to the conglomerate around them.

Perhaps it's sometimes a bit harder to maintain when
<!ENTITY longerthan_longername "bla">
is added and you might end up reformatting the whole set.
However, having a few variants of vertical alignments imo doesn't hurt.
We could find a reasonable default horizontal offset position which is wide enough for most regular entity names, as in the IMchat example above.

Especially if localizers are actually editing the dtd-files in source, I recommend to at least keep format 2 where we currently have it (whereas kunal's current patch uses Format 1, i.e. he removed the vertical value alignment to make all entries structurally consistent).

*** Malformat 3:
<!ENTITY name     "somevalue">
<!ENTITY longername "someothervalue">

This is neither Format 1 nor Format 2, hence should be fixed.

Whatever we want changed here or not (compared to the current mix of formats) needs specific and clear directions. Magnus?

Comment 34

[Thomas D. (:thomas8)]

^^ @aryx: needs your information for comment 33, section 3) ***** sendProgress.dtd

Comment 35

[Thomas D. (:thomas8)]

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

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

*** Disclaimer: This is NOT a review (just using splinter for feedback)

> # HG changeset patch
> # Parent 8b432fa340ec64ce63eca71fe6c19f83dd3655a3
> # User Ryan VanderMeulen <[e-mail]>
> BUG802046:
> This is the final patch
> Please commit the changes after final verification.
> Thankyou
> * * *

Kunal, I think on the patch header, we want the user name of the patch author (you), not the name of the committer (Ryan). You can set up your build to include your name for each patch automatically, as described here:

https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

As a description in patch header, pls do NOT add personal messages for committing (like "now ready for checkin").
You should put Bug number and a short version of Bug summary, so that posterity on hg knows what this patch is about, like this:

Bug 802046 - Typos, whitespace and similar nitfixes in localization/l10n comments and string entities

::: mail/locales/en-US/chrome/messenger/messenger.dtd
@@ +456,5 @@
>  <!ENTITY imAccountsStatus.accesskey "C">
> +<!ENTITY imStatus.available "Available">
> +<!ENTITY imStatus.unavailable "Unavailable">
> +<!ENTITY imStatus.offline "Offline">
> +<!ENTITY imStatus.showAccounts "Show Accounts…">

Let's wait for Magnus' comment before removing this whitespace originally added for vertical alignment of values for better human readability (for details, see comment 33).

@@ +720,5 @@
>  <!ENTITY columnPicker.resetToInbox.label "Reset columns to default">
>  <!-- LOCALIZATION NOTE (columnPicker.applyTo.label):
>     This option in the thread pane column picker pops up a sub-menu containing
>     the "columnPicker.applyToFolder.label" and
> +   "columnPicker.applyToFolderAndChildren.label" options. This item indicates

Removing the double spaces after full stops looks all right to me.

@@ +888,3 @@
>  <!ENTITY hideOtherAppsCmdMac.commandkey "H">
> +<!ENTITY hideOtherAppsCmdMac.modifiers "accel,alt">
> +<!ENTITY showAllAppsCmdMac.label "Show All">

Readability really gets worse by this change, so I propose to keep this as is. For details, see my comment 33.

::: suite/locales/en-US/chrome/mailnews/FilterEditor.dtd
@@ +55,5 @@
>    The values below are used to control the widths of the filter action widgets.
>    Change the values only when the localized strings in the popup menus
>    are truncated in the widgets.
>   -->
> +<!-- Flex Attribute: https://developer.mozilla.org/en-US/docs/XUL/Attribute/flex -->

This looks good to me. I just wonder if we should drop en-US from the path so that the link redirects to the localized versions if these are available?

Comment 36

[Magnus Melin [:mkmelin]]

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

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

Looks good to me

Comment 37

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. from comment #35)
> ::: mail/locales/en-US/chrome/messenger/messenger.dtd
> @@ +456,5 @@
> >  <!ENTITY imAccountsStatus.accesskey "C">
> > +<!ENTITY imStatus.available "Available">
> > +<!ENTITY imStatus.unavailable "Unavailable">
> > +<!ENTITY imStatus.offline "Offline">
> > +<!ENTITY imStatus.showAccounts "Show Accounts…">
> 
> Let's wait for Magnus' comment before removing this whitespace originally
> added for vertical alignment of values for better human readability (for
> details, see comment 33).

I think one space is enough. Otherwise over time there's always that one long thing that makes it not fit anyways.

> 
> @@ +720,5 @@
> >  <!ENTITY columnPicker.resetToInbox.label "Reset columns to default">
> >  <!-- LOCALIZATION NOTE (columnPicker.applyTo.label):
> >     This option in the thread pane column picker pops up a sub-menu containing
> >     the "columnPicker.applyToFolder.label" and
> > +   "columnPicker.applyToFolderAndChildren.label" options. This item indicates
> 
> Removing the double spaces after full stops looks all right to me.
> 
> @@ +888,3 @@
> >  <!ENTITY hideOtherAppsCmdMac.commandkey "H">
> > +<!ENTITY hideOtherAppsCmdMac.modifiers "accel,alt">
> > +<!ENTITY showAllAppsCmdMac.label "Show All">
> 
> Readability really gets worse by this change, so I propose to keep this as
> is. For details, see my comment 33.
> 
> ::: suite/locales/en-US/chrome/mailnews/FilterEditor.dtd
> @@ +55,5 @@
> >    The values below are used to control the widths of the filter action widgets.
> >    Change the values only when the localized strings in the popup menus
> >    are truncated in the widgets.
> >   -->
> > +<!-- Flex Attribute: https://developer.mozilla.org/en-US/docs/XUL/Attribute/flex -->
> 
> This looks good to me. I just wonder if we should drop en-US from the path
> so that the link redirects to the localized versions if these are available?

Comment 38

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. from comment #33)
> I think we need to improve this more than just removing " - see".
> I propose this as a minimal fix replacement:
> 
> 43 ## LOCALIZATION NOTE(filterCountItems): Semicolon-separated list of
> singular and plural forms.
> 44 ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> 45 ## #1 is the count of items in the list.
> 46 filterCountItems=#1 item; #1 items
> 
> Magnus, pls confirm if that's ok.

Ok by me.
 
> This needs clearer instructions which whitespaces we want removed or not.

Well, i think it's actually not very good use of anyones time to fiddle with these...

> I suggest the following indentation for localization comments (which seems
> to be the current default format):
> 
> 19 <!-- LOCALIZATION NOTE (moveToNewWindow.label):
> 20      Menu option to cause the current tab to be migrated to a new
> Thunderbird
> 21      window.
> 22      -->

The format with --s lined up seems common and perhaps the best. But like i said, there's more important things to do, so let's just keep them as they are, where not very very wrong.

Comment 39

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #38)
> (In reply to Thomas D. from comment #33)
> > I think we need to improve this more than just removing " - see".
> > I propose this as a minimal fix replacement:
> > 
> > 43 ## LOCALIZATION NOTE(filterCountItems): Semicolon-separated list of
> > singular and plural forms.
> > 44 ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> > 45 ## #1 is the count of items in the list.
> > 46 filterCountItems=#1 item; #1 items
> > 
> > Magnus, pls confirm if that's ok.
> 
> Ok by me.

kunal, would you want to add that change in filter.properties file to your patch?

http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/filter.properties#43

Comment 40

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

(In reply to Thomas D. from comment #33)
> 3) ***** sendProgress.dtd *****
> 
> > messengerCompose/sendProgress.dtd:
> > 
> > <!--LOCALIZATION NOTE sendprogress.dtd Main UI for Send Message Progress
> > Dialog -->
> > <!ENTITY sendDialog.title "Processing Message">
> > 
> > sendprogress.dtd has been removed.
> 
> :aryx, can you explain what exactly you wanted to change here? I don't
> understand. Both sendprogress.xul and sendprogress.dtd still exist.
> 
I don't remember, maybe I got confused by the lowercase 'p' in the file name where it should be uppercase.

> 4) ***** messenger.dtd *****
> 4d) More than one whitespace in <!ENTITY name "value">
> Especially if localizers are actually editing the dtd-files in source, I
> recommend to at least keep format 2 where we currently have it (whereas
> kunal's current patch uses Format 1, i.e. he removed the vertical value
> alignment to make all entries structurally consistent).
> 
I had localized Thunderbird for some time (and still do it for the Firefoxes), and the value of aligning the entities value is near nil. Either the adjacent entities get translated in a batch and you know the context or you will check for yourself what the adjacent entities are when adding an entity later.

Comment 41

[kunal bansal]

Attachment: patch

I have made all the changes as suggested above.
The template followed for the localization note is:
<!-- LOCALIZATION NOTE (columnPicker.resetToInbox.label):
   This option in the thread pane column picker causes us to reset the
   customizations for the thread pane columns in this folder to their default.
  -->
please verify if i'm correct.

Comment 42

[kunal bansal]

I believe there should be a quick to read abstract somewhere which specifies these indentation rules.

Comment 43

[Magnus Melin [:mkmelin]]

(In reply to kunal bansal from comment #41)
> <!-- LOCALIZATION NOTE (columnPicker.resetToInbox.label):
>    This option in the thread pane column picker causes us to reset the
>    customizations for the thread pane columns in this folder to their
> default.
>   -->
> please verify if i'm correct.

That looks like one space off

<!-- LOCALIZATION NOTE (columnPicker.resetToInbox.label):
     This option in the thread pane column picker causes us to reset the
     customizations for the thread pane columns in this folder to their default.
  -->

or

<!-- LOCALIZATION NOTE (columnPicker.resetToInbox.label):
  This option in the thread pane column picker causes us to reset the
  customizations for the thread pane columns in this folder to their default.
  -->

look better imho. but like a said earlier, there's no real need to change these

Comment 44

[kunal bansal]

okay so should i remove all alterations related to localization.to me also it
seems irrelevant to spend time on this when you have much better work to do

Comment 45

[Magnus Melin [:mkmelin]]

yes, or fix those you already touched to be in one of the above formats. i'll leave that up to you

Comment 46

[kunal bansal]

Attachment: patch

this follows the rule 
<!-- LOCALIZATION NOTE (columnPicker.resetToInbox.label):
     This option in the thread pane column picker causes us to reset the
     customizations for the thread pane columns in this folder to their default.
  -->

Comment 47

[Magnus Melin [:mkmelin]]

Comment on attachment 751343 [details] [diff] [review]
patch

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

Unfortunately the patch doesn't seem to apply.

applying bug802046.patch
patching file mail/locales/en-US/chrome/messenger/messenger.dtd
Hunk #1 FAILED at 0
1 out of 7 hunks FAILED -- saving rejects to file mail/locales/en-US/chrome/messenger/messenger.dtd.rej
patch failed, unable to continue (try -v)

::: mail/locales/en-US/chrome/messenger/filter.properties
@@ +41,5 @@
>  # %1$S=number of matching filters, %2$S=total number of filters
>  filterCountVisibleOfTotal=%1$S of %2$S
>  ## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
>  ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> +## #1 is the count of items in the list. 

nit: remove trailing white space

Comment 48

[kunal bansal]

Attachment: New patch

the error has been removed

Comment 49

[Thomas D. (:thomas8)]

(In reply to kunal bansal from comment #48)
> Created attachment 751702 [details] [diff] [review]
> New patch
> the error has been removed

> -## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.

Sorry, but as this bug is all about nitfixes, I still think that we should fix wrong spelling and content while we are here, as approved by Magnus in Comment 38:
- Semi-colon is misspelled -> Semicolon
- "Semi-colon list" is odd -> Semicolon-separated list (as in comma-separated list)
- it's not a list of *plural* forms, but a list of singular *and* plural forms

So the patch should include this:

-## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
+## LOCALIZATION NOTE(filterCountItems): Semicolon-separated list of singular and plural forms.

(without linebreak; there are plenty of much longer lines in that file)

> ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
>-## #1 is the count of items in the list.# filterCountItems is a pluralForm - see 
>+## #1 is the count of items in the list.

That part is ok now.

Comment 50

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/a8ea2881621a20f5d52a5c32ba8dee476231dc01
Bug 802046 - Typos, whitespace and similar nitfixes in localization/l10n comments and string entities. r=mkmelin a=cleanup DONTBUILD