Last Comment Bug 858238 - localMsgs.properties should use string based identifiers rather than numbers
: localMsgs.properties should use string based identifiers rather than numbers
Status: RESOLVED FIXED
[good first bug][mentor=aceman][lang=...
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 24.0
Assigned To: Suyash Agarwal (:sshagarwal)
:
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on:
Blocks: 221592 856577
  Show dependency treegraph
 
Reported: 2013-04-04 13:59 PDT by :aceman
Modified: 2013-05-15 05:14 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (66.40 KB, patch)
2013-04-11 15:00 PDT, Suyash Agarwal (:sshagarwal)
no flags Details | Diff | Splinter Review
patch v2 (67.95 KB, patch)
2013-04-12 14:15 PDT, :aceman
no flags Details | Diff | Splinter Review
Patch (51.13 KB, patch)
2013-04-15 03:36 PDT, Suyash Agarwal (:sshagarwal)
no flags Details | Diff | Splinter Review
Patch (indented) (70.62 KB, patch)
2013-04-16 12:03 PDT, Suyash Agarwal (:sshagarwal)
neil: review-
acelists: feedback+
Details | Diff | Splinter Review
Patch (72.20 KB, patch)
2013-05-01 12:23 PDT, Suyash Agarwal (:sshagarwal)
neil: feedback+
Details | Diff | Splinter Review
Patch v2 (72.61 KB, patch)
2013-05-04 13:18 PDT, Suyash Agarwal (:sshagarwal)
neil: review+
Details | Diff | Splinter Review
Patch v3 (74.02 KB, patch)
2013-05-14 08:17 PDT, Suyash Agarwal (:sshagarwal)
syshagarwal: review+
Details | Diff | Splinter Review
Patch v4 (74.04 KB, patch)
2013-05-14 14:27 PDT, Suyash Agarwal (:sshagarwal)
syshagarwal: review+
acelists: feedback+
Details | Diff | Splinter Review

Description :aceman 2013-04-04 13:59:28 PDT
localMsgs.properties currently uses numbers for its identifiers rather than strings. This makes it harder for localisers to work with, and means that if we need to tweak strings then it isn't easy to change the string identifier to notify the localisers.

We should work on replacing the numbers with strings.

This affects POP3 and Movemail protocols.

Coding hint:
FormatStringFromID should be replaced with FormatStringFromName, etc.
Comment 1 Derek Perrin 2013-04-08 21:01:40 PDT
Hi, I'm new to firefox development and would like to take this as my first bug.
Comment 2 :aceman 2013-04-08 23:31:40 PDT
This bug is about Thunderbird or Seamonkey. Do you want to work on it?
Comment 3 Derek Perrin 2013-04-09 07:08:14 PDT
I noticed that when I started to look for stuff. I'd still like to work on it though.
Comment 4 :aceman 2013-04-09 07:29:04 PDT
OK, do you have the Thunderbird build environment set up?
Comment 5 Derek Perrin 2013-04-09 07:36:40 PDT
I do, and I've built Thunderbird. Are you on IRC at all?
Comment 6 :aceman 2013-04-09 07:43:04 PDT
Yes, I'll be on #maildev in 4 hours from now.
Comment 7 :aceman 2013-04-09 10:59:28 PDT
So, this needs to be done here:
1. in the file mail/locales/en-US/chrome/messenger/localMsgs.properties look for string definitions like this:
# Status - parsing folder
## @name LOCAL_STATUS_SELECTING_MAILBOX
## @loc None
#LOCALIZATION NOTE (4000): Do not translate %s in the following line.
# Place the word %s where the name of the mailbox should appear
4000=Building summary file for %S…

2A. Change it to having a string identifier, like this:
buildingSummary=Building summary file for %S…

2B. Do the same in the equivalent Seamonkey file suite/locales/en-US/chrome/mailnews/localMsgs.properties .

3. Search for the LOCAL_STATUS_SELECTING_MAILBOX constant in the tree: http://mxr.mozilla.org/comm-central/search?string=LOCAL_STATUS_SELECTING_MAILBOX&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

4. Remove the "#define LOCAL_STATUS_SELECTING_MAILBOX 4000"

5. Find all usages of LOCAL_STATUS_SELECTING_MAILBOX and replace with "buildingSummary". All functions where the new identifier is passed should have the argument changed from int to PRUnichar* or something (see how FormatStringFromName is used in the tree).
Also check whether the original numeric ID (4000) is used directly somewhere, not via the LOCAL_STATUS_SELECTING_MAILBOX define. Also change those occurrences to the new string identifier.

6. Where the original numeric ID (via the define) was passed to FormatStringFromID, change the function to FormatStringFromName.
Comment 8 Magnus Melin 2013-04-09 12:26:37 PDT
It might be a good(?) idea to use the names as new key. (E.g. LOCAL_STATUS_SELECTING_MAILBOX=Building summary file for %S…) At least don't make the keys too short or too generic to find.
Comment 9 Suyash Agarwal (:sshagarwal) 2013-04-11 15:00:01 PDT
Created attachment 736513 [details] [diff] [review]
Patch

Sir, 
Thanks, I have attached the patch. Looking forward for your comments.
Comment 10 :aceman 2013-04-11 15:08:51 PDT
Thanks, I will look at the patch soon, please wait for the comments.
Comment 11 :aceman 2013-04-12 10:06:08 PDT
Sorry, but this does not compile for me. I get tons of errors like this:
mailnews/local/src/nsPop3Protocol.cpp: In member function 'int32_t nsPop3Protocol::Error(PRUnichar*)':
mailnews/local/src/nsPop3Protocol.cpp:1260:21: warning: comparison with string literal results in unspecified behaviour [-Waddress]
mailnews/local/src/nsPop3Protocol.cpp:1260:21: error: comparison between distinct pointer types 'PRUnichar* {aka short unsigned int*}' and 'const char*' lacks a cast [-fpermissive]
mailnews/local/src/nsPop3Protocol.cpp:1287:115: error: no matching function for call to 'nsIStringBundle::FormatStringFromName(const char [16], const PRUnichar* [1], int, nsGetterCopies)'
mailnews/local/src/nsPop3Protocol.cpp:1287:115: note: candidate is:
In file included from /var/SSD/TB-hg/mailnews/local/src/nsPop3Protocol.h:18:0,
                 from /var/SSD/TB-hg/mailnews/local/src/nsPop3Protocol.cpp:28:
../../../mozilla/dist/include/nsIStringBundle.h:55:60: note: virtual nsresult nsIStringBundle::FormatStringFromName(const PRUnichar*, const PRUnichar**, uint32_t, PRUnichar**)
../../../mozilla/dist/include/nsIStringBundle.h:55:60: note:   no known conversion for argument 1 from 'const char [16]' to 'const PRUnichar* {aka const short unsigned int*}'

The first argument of nsPop3Protocol::Error probably can't be PRUnichar* so easily.
We need to find out what the type should be so that it accepts plain char*. There is no other ::Error method where we could look how it is done. Neil?
Comment 12 :aceman 2013-04-12 10:18:53 PDT
Should we make it nsACString and send the string identifiers as NS_LITERAL_CSTRING("string")? But that may be overkill.
Comment 13 Joshua Cranmer [:jcranmer] 2013-04-12 10:28:32 PDT
(In reply to :aceman from comment #11)
> The first argument of nsPop3Protocol::Error probably can't be PRUnichar* so
> easily.
> We need to find out what the type should be so that it accepts plain char*.
> There is no other ::Error method where we could look how it is done. Neil?

NS_LITERAL_STRING("").get() is how you call those sorts of methods.
Comment 14 Suyash Agarwal (:sshagarwal) 2013-04-12 11:08:42 PDT
Sir,
So, what should I do then? I mean, what should PRUnichar* be replaced with, and yes, at some places, strings are compared, so should I use the c function strcmp() ? 
And, what is "Neil"? Its your last mentioned word in your comment #11
Comment 15 :aceman 2013-04-12 11:40:37 PDT
(In reply to Suyash Agarwal from comment #14)
> Sir,
> So, what should I do then? I mean, what should PRUnichar* be replaced with,
> and yes, at some places, strings are compared, so should I use the c
> function strcmp() ? 
Keep the PRUnichar* but replace the strings with NS_LITERAL_STRING("").get() as jcranmer says. So e.g.:
if (err_code != "downloadError")
change to:
if (err_code != NS_LITERAL_STRING("downloadError").get())

Change
Error("pop3ServerError")
to
Error(NS_LITERAL_STRING("pop3ServerError").get())

And please try to compile the patch after you do this. It seems you haven't done it the first time before uploading.

> And, what is "Neil"? Its your last mentioned word in your comment #11
That is the name of one of our gurus :)
Comment 16 :aceman 2013-04-12 11:45:07 PDT
jcranmer, thanks for the help. Shouldn't we then make Error take nsString and only call the .get() when calling FormatStringFromName ?
Comment 17 Joshua Cranmer [:jcranmer] 2013-04-12 11:54:07 PDT
(In reply to :aceman from comment #15)
> Keep the PRUnichar* but replace the strings with NS_LITERAL_STRING("").get()
> as jcranmer says. So e.g.:
> if (err_code != "downloadError")
> change to:
> if (err_code != NS_LITERAL_STRING("downloadError").get())

That code is very wrong--you cannot compare string literals with ==/!= in C++. This makes me think there may be some more pernicious things going on around that call, but I haven't read most of the patch.
Comment 18 neil@parkwaycc.co.uk 2013-04-12 11:56:12 PDT
(In reply to aceman from comment #15)
> if (err_code != "downloadError")
> change to:
> if (err_code != NS_LITERAL_STRING("downloadError").get())
No! don't do either of those!

(In reply to aceman from comment #16)
> jcranmer, thanks for the help. Shouldn't we then make Error take nsString
> and only call the .get() when calling FormatStringFromName ?
In theory it's easier for the compiler to optimse if you pass the PRUnichar.
Comment 19 :aceman 2013-04-12 11:58:52 PDT
OK, some version of err_code.Equals(NS_LITERAL_STRING("downloadError")) ?
Comment 20 Suyash Agarwal (:sshagarwal) 2013-04-12 12:02:58 PDT
Oh okay sir. But why can't I use strcmp() to compare strings?? And, what does this NS_LITERAL_STRING ("").get() actually do? Does it produce an equivalent integer to be compared?

And, what should I do in situations of the terenary operators, and few comparisons of the form: err_code != "downloadError"? Here, should I replace this "downloadError" also with NS_LITERAL_STRING("downloadError").get() or, strcmp()? And same for the error for FormatStringFromName(), should its parameter also be replaced with NS_LITERAL_STRING("").get() ??
Comment 21 neil@parkwaycc.co.uk 2013-04-12 12:10:38 PDT
There's a similar problem in nsMsgMailboxParser::UpdateStatusText, which happens to insert the folder name if the string is the selecting mailbox string. As there are only the two strings to worry about, perhaps a different approach is called for in that case.
Comment 22 :aceman 2013-04-12 14:15:42 PDT
Created attachment 736978 [details] [diff] [review]
patch v2

Neil, jcranmer, is this what you meant? I fixed up nsPop3Protocol.cpp so that is compiles now.
Comment 23 Suyash Agarwal (:sshagarwal) 2013-04-15 03:36:51 PDT
Created attachment 737422 [details] [diff] [review]
Patch

Sir,
I have attached a patch, and this compiles and builds properly.
Waiting for the comments.
Comment 24 :aceman 2013-04-15 03:41:39 PDT
I wonder how your patch is so much smaller than mine. But I'll check it out.
Comment 25 :aceman 2013-04-15 12:41:09 PDT
Comment on attachment 737422 [details] [diff] [review]
Patch

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

Good work now! The patch compiles and I have tested the "Building summary file" string that it does appear on the status line when needed.

I've just marked some style problems, as written below. I will not comment on the chosen names of the strings, that will be decided by the real reviewer.

::: mailnews/local/src/nsLocalMailFolder.cpp
@@ +845,1 @@
>                                     getter_Copies(deleteFolderDialogTitle));

Please add 2 spaces so that 'getter' aligns below 'NS_'. This happens in several places.

@@ +850,1 @@
>                                     getter_Copies(deleteFolderButtonLabel));

E.g. here.

@@ +3165,1 @@
>                                                 getter_Copies(finalString));

Can 'getter' be aligned below 'status' ?

::: mailnews/local/src/nsLocalStrings.h
@@ +47,2 @@
>  
>  #endif /* _nsLocalStrings_H__ */

Can you remove the whole file altogether and remove its inclusion where it is referenced?

::: mailnews/local/src/nsParseMailbox.cpp
@@ +119,5 @@
>              // the size of the mailbox file is our total base line for measuring progress
>              m_graph_progress_total = (uint32_t) fileSize;
> +            //UpdateStatusText(LOCAL_STATUS_SELECTING_MAILBOX);
> +	    UpdateStatusText(NS_LITERAL_STRING("buildingSummary").get());
> +	    //UpdateStatusText(NS_LITERAL_STRING("buildingSummary").get());	

Can you leave only the correct line and remove the commented code?

@@ +177,5 @@
>      // be sure to clear any status text and progress info..
>      m_graph_progress_received = 0;
>      UpdateProgressPercent();
> +//    UpdateStatusText(LOCAL_STATUS_DOCUMENT_DONE);
> +    UpdateStatusText (NS_LITERAL_STRING("done").get());

Remove the commented out line.

@@ +301,1 @@
>                                        getter_Copies(finalString));

Align 'string' and 'getter'.

::: mailnews/local/src/nsParseMailbox.h
@@ +168,5 @@
>  
>    void  UpdateDBFolderInfo();
>    void  UpdateDBFolderInfo(nsIMsgDatabase *mailDB);
> +//  void  UpdateStatusText (uint32_t stringID);
> +  void UpdateStatusText (const PRUnichar* stringName);

Remove commented line.

::: mailnews/local/src/nsPop3Protocol.h
@@ +45,4 @@
>   */
>  
>  #define MK_OUT_OF_MEMORY -207
> +// #define MK_POP3_OUT_OF_DISK_SPACE -321

Remove the line altogether.
Comment 26 :aceman 2013-04-15 12:43:46 PDT
Oh, the review was produced strangely. I'll write the first comments again if they can't be understood.

Also, please do not use tabs in the code. Use normal spaces instead.
Comment 27 :aceman 2013-04-15 12:49:07 PDT
--- a/mailnews/local/src/nsLocalMailFolder.cpp<>Mon Apr 08 00:56:01 2013 +0100
+++ b/mailnews/local/src/nsLocalMailFolder.cpp<>Mon Apr 15 21:31:15 2013 +0530
@@ -841,17 +841,17 @@
       const PRUnichar *formatStrings[1] = { folderName.get() };
.
       nsAutoString deleteFolderDialogTitle;
-      rv = bundle->GetStringFromID(POP3_DELETE_FOLDER_DIALOG_TITLE,
+      rv = bundle->GetStringFromName(NS_LITERAL_STRING("deleteFolderDialog").get(),
                                    getter_Copies(deleteFolderDialogTitle));

The first 3 remarks wanted to write that you should use spaces to align "getter_Copies" under "NS_LITERAL_STRING".
Comment 28 Suyash Agarwal (:sshagarwal) 2013-04-16 12:03:50 PDT
Created attachment 738119 [details] [diff] [review]
Patch (indented)

Sir, the patch is modifies as instructed. Waiting for the comments.
Comment 29 :aceman 2013-04-16 12:53:04 PDT
Comment on attachment 738119 [details] [diff] [review]
Patch (indented)

Yes, this looks good now! You still left in some tabs, so please remove them in the next version of the patch, that you create when the real reviewer writes his comments. I'm forwarding to Neil for next review.
Comment 30 neil@parkwaycc.co.uk 2013-04-17 12:47:08 PDT
Comment on attachment 738119 [details] [diff] [review]
Patch (indented)

>-4043=Unable to establish TLS connection to POP3 server. The server may be down or may be incorrectly configured. Please verify the correct configuration in the Server Settings for your mail server in the Account Settings window and try again.
>+COULD_NOT_CONNECT_VIA_TLS=Unable to establish TLS connection to POP3 server. The server may be down or may be incorrectly configured. Please verify the correct configuration in the Server Settings for your mail server in the Account Settings window and try again.
[Someone (not me!) needs to come up with some better names...]

>       (void) ThrowAlertMsg((rv == NS_MSG_FOLDER_EXISTS) ?
>-                            "folderExists" : "folderRenameFailed", msgWindow);
>+                           "folderExists" : 
>+			   "folderRenameFailed",
>+			   msgWindow);
Why did this change?

>+      const PRUnichar* statusMsgName = ((mCopyState->m_isMove) ? NS_LITERAL_STRING("movingMessages")
>+		      					       : NS_LITERAL_STRING("copyingMessages")).get();
Sorry, but the NS_LITERAL_STRING gets deleted at the end of the statement, so you can't use .get() here. What would work though would be to write the ?: expression directly in the FormatStringFromName call.

>+    if (nsDependentString(stringName).EqualsLiteral("buildingSummary"))
I'm not that keen on using a string comparison here, but I couldn't think of a good way to avoid it.

>+    PR_LOG(POP3LOGMODULE, PR_LOG_ALWAYS, ("ERROR: %s", err_code));
Sorry, but PR_LOG only works on const char* strings. I suppose you could change err_code to be a char* and then convert it to UTF16 later (compare nsMsgDBFolder::GetStringWithFolderNameFromBundle).

>-    if (err_code != POP3_TMP_DOWNLOAD_FAILED && NS_SUCCEEDED(rv))
>+    // we handle "downloadError" earlier...
>+    if (nsDependentString(err_code).EqualsLiteral("downloadError") && NS_SUCCEEDED(rv))
I think you accidentally deleted the !
Also, I don't like the string comparison again, but I still don't have a better idea.

>-    // XXX Error() returns -1, which is not a valid nsresult
>-    return static_cast<nsresult>(Error(POP3_USERNAME_UNDEFINED));
>+    Error(NS_LITERAL_STRING("usernameUndefined").get());
>+    return NS_MSG_SERVER_USERNAME_MISSING;
Nice!
Comment 31 Suyash Agarwal (:sshagarwal) 2013-05-01 12:23:33 PDT
Created attachment 744233 [details] [diff] [review]
Patch

Sir, 

I have tried to address all the issues mentioned by you.
Please review this patch.
Comment 32 neil@parkwaycc.co.uk 2013-05-01 13:50:01 PDT
Comment on attachment 744233 [details] [diff] [review]
Patch

This is looking good. Has it been tested on the Try server? There are just a few things that I would like to resolve:

> ## @name MK_POP3_OUT_OF_DISK_SPACE
> ## @loc None
Sorry for not noticing before, but I think the patch probably makes all of these @name/@loc lines unnecessary, and you could remove them.

>-    if (stringID == LOCAL_STATUS_SELECTING_MAILBOX)
>+    if (nsDependentString(stringName).EqualsLiteral("buildingSummary"))
>     {
>       const PRUnichar * stringArray[] = { m_folderName.get() };
>-      rv = bundle->FormatStringFromID(stringID, stringArray, 1,
>-                                      getter_Copies(finalString));
>+      rv = bundle->FormatStringFromName(stringName, stringArray, 1,
>+                                        getter_Copies(finalString));
>     }
>     else
>-      bundle->GetStringFromID(stringID, getter_Copies(finalString));
>+      bundle->GetStringFromName(stringName, getter_Copies(finalString));
There was a suggestion on IRC that you could avoid the string comparison by always formatting the string (the strings that didn't need the format would just ignore it).

>+    PR_LOG(POP3LOGMODULE, PR_LOG_ALWAYS, 
>+          ("ERROR: %s", NS_LossyConvertUTF16toASCII(err_code).get()));
Seeing as you always need to do a conversion, it might be a better idea to use a const char* err_code instead, and then convert it to UTF16 when getting the string. That advantage is that you don't have to fiddle around with NS_LITERAL_STRING().get() all the time.

>+            return(Error(((m_password_already_sent)
>+                          ? NS_LITERAL_STRING("pop3PasswordFailure")
>+                          : NS_LITERAL_STRING("pop3UsernameFailure")).get()));
Another thing that I overlooked before is that I think the Mozilla coding style now prefers the ? and : operators at the end of the previous line. Also, I think earlier on in the patch you used a separate .get() for each NS_LITERAL_STRING; I prefer that version, so the code I quoted would end up something like this:
return Error(m_password_already_sent ?
             NS_LITERAL_STRING("pop3PasswordFailure").get() :
             NS_LITERAL_STRING("pop3UsernameFailure").get());
I think there is at least one other case of ( ? : ).get() that should be updated.
Comment 33 :aceman 2013-05-01 14:12:21 PDT
Thanks, hopefully we are nearing finish here.
Comment 34 Suyash Agarwal (:sshagarwal) 2013-05-04 13:18:30 PDT
Created attachment 745577 [details] [diff] [review]
Patch v2

Sir,

I have made the changes suggested. Waiting for your comments.
Comment 35 neil@parkwaycc.co.uk 2013-05-13 15:45:01 PDT
Comment on attachment 745577 [details] [diff] [review]
Patch v2

This looks good, assuming all the tests pass. (Has anyone run them?)

(It would be nice of the @name and @loc lines could be removed from suite/locales/en-US/chrome/mailnews/localMsgs.properties too.)
Comment 36 Suyash Agarwal (:sshagarwal) 2013-05-14 08:17:05 PDT
Created attachment 749298 [details] [diff] [review]
Patch v3

Sir,
Sorry I left the suite/ file in the previous patch. I have made the changes in this one.
:aceman sir, please provide the feedback so that I can mark this for check-in.
Carrying over review from Neil.
Comment 37 :aceman 2013-05-14 09:48:22 PDT
Comment on attachment 749298 [details] [diff] [review]
Patch v3

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

Please update your tree as this patch surely will not apply cleanly due to checkin of bug 640371. Then please update the patch. As you are going to change the patch anyway I have some nits.

::: mailnews/local/src/nsMovemailService.cpp
@@ -114,5 @@
>    if (NS_FAILED(rv))
>      return;
>  
>    nsString errStr;
> -  // Format the error string if necessary

Why remove this comment?

::: mailnews/local/src/nsParseMailbox.cpp
@@ -176,5 @@
>      // be sure to clear any status text and progress info..
>      m_graph_progress_received = 0;
>      UpdateProgressPercent();
> -    UpdateStatusText(LOCAL_STATUS_DOCUMENT_DONE);
> -

Do not remove the empty line and do not add space before ( .

::: mailnews/local/src/nsParseMailbox.h
@@ -168,5 @@
>  
>    void  UpdateDBFolderInfo();
>    void  UpdateDBFolderInfo(nsIMsgDatabase *mailDB);
> -  void  UpdateStatusText (uint32_t stringID);
> -

Probably keep the blank line here.

::: mailnews/local/src/nsPop3Protocol.cpp
@@ +2343,5 @@
>        if (NS_FAILED(rv))
>        {
>          m_nsIPop3Sink->AbortMailDelivery(this);
>          if (rv == NS_MSG_FOLDER_BUSY)
> +          return Error("pop3MessageFolderBusy");

Can this be changed to the return Error(rv="xxx"? "error1" : "error2") style we already used elsewhere?

@@ +2985,5 @@
>            // Not enough disk space!
>  #ifdef DEBUG
>            printf("Not enough disk space! Raising error! \n");
>  #endif
> +          return Error("pop3OutOfDiskSpace");

This will need update due to the clash with bug 640371.
Comment 38 :aceman 2013-05-14 13:13:42 PDT
(In reply to neil@parkwaycc.co.uk from comment #35)
> This looks good, assuming all the tests pass. (Has anyone run them?)

Yes, I've run TB tests on linux (xpcshell and mozmill) with this.
Comment 39 Suyash Agarwal (:sshagarwal) 2013-05-14 14:27:08 PDT
Created attachment 749481 [details] [diff] [review]
Patch v4

Sir,

I have made the changes you suggested.
Carrying over review from Neil.
Comment 40 :aceman 2013-05-14 14:46:41 PDT
Comment on attachment 749481 [details] [diff] [review]
Patch v4

Great, everything is done:)
Comment 41 :aceman 2013-05-14 14:48:40 PDT
Let's drive this in finally, other bugs are waiting in queue for it to land :)
Congrats to your first taken bug being fixed. And it ended up to be quite big patch.
Comment 42 Suyash Agarwal (:sshagarwal) 2013-05-14 14:52:28 PDT
Thank you sir :)
Its my pleasure learning from you.
Comment 43 Ryan VanderMeulen [:RyanVM] 2013-05-15 05:14:41 PDT
https://hg.mozilla.org/comm-central/rev/41a231cc4161

Note You need to log in before you can comment on or make changes to this bug.