Closed Bug 232691 Opened 16 years ago Closed 10 years ago

replace nsString() nsAutoString() and friends with EmptyC?String()

Categories

(Core :: General, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: Biesinger, Assigned: ewong)

Details

(Whiteboard: [good first bug])

Attachments

(12 files, 15 obsolete files)

78.51 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
7.91 KB, patch
mconnor
: review+
Details | Diff | Splinter Review
22.09 KB, patch
bzbarsky
: superreview+
Details | Diff | Splinter Review
5.85 KB, patch
Details | Diff | Splinter Review
23.98 KB, patch
vhaarr+bmo
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
3.14 KB, patch
sicking
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
610 bytes, patch
sicking
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
574 bytes, patch
Details | Diff | Splinter Review
2.29 KB, patch
Details | Diff | Splinter Review
1.02 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.02 KB, patch
ehsan
: review+
Details | Diff | Splinter Review
861 bytes, patch
Bienvenu
: review-
Details | Diff | Splinter Review
now that we have EmptyString(), we should replace things like nsString() and
nsAutoString() with EmptyString().

(most of the lxr hits are such hits... I couldn't figure out a query that only
finds what I wanted)
Whiteboard: [good first bug]
OS: Linux → All
Hardware: PC → All
There are also a few NS_LITERAL_C?STRING("") that have snuck into the code again...
Attached patch diff -u8 (obsolete) — Splinter Review
Right now all I know is that this compiles...
Attached patch diff-u8 (obsolete) — Splinter Review
Forgot to save the removal of extraneous items before uploading.  Doh.
Attachment #149092 - Attachment is obsolete: true
Comment on attachment 149093 [details] [diff] [review]
diff-u8

>Index: accessible/src/html/nsHTMLFormControlAccessible.cpp
>-        _retval.Assign(NS_LITERAL_STRING(""));  // Default name is blank 
>+        _retval.Assign(EmptyString());  // Default name is blank 

|_retval.Truncate();| is better here.

>Index: accessible/src/html/nsHTMLTableAccessible.cpp

>-  aResult.Assign(NS_LITERAL_STRING(""));  // Default name is blank
>+  aResult.Assign(EmptyString());  // Default name is blank

aResult.Truncate();

> NS_IMETHODIMP nsHTMLTableAccessible::GetName(nsAString& aResult)
> {
>-  aResult.Assign(NS_LITERAL_STRING(""));  // Default name is blank
>+  aResult.Assign(EmptyString());  // Default name is blank

Again.

>Index: accessible/src/xul/nsXULFormControlAccessible.cpp

>-  _retval.Assign(NS_LITERAL_STRING(""));  // Default name is blank 
>+  _retval.Assign(EmptyString());  // Default name is blank 

Here too.

Could you skim over the patch and fix up places that do this to use Truncate?

We should only need EmptyC?String() when passing an empty string to a callee...
Attachment #149093 - Attachment is obsolete: true
Assignee: general → clf03f
Comment on attachment 149095 [details] [diff] [review]
latest and greatest
[Checkin: Comment 7]

r+sr=bzbarsky.	For future  reference, you want to request reviews by setting
the flags on the patch accordingly (and filling in the bugmail of the person
whose review you want).
Attachment #149095 - Flags: superreview+
Attachment #149095 - Flags: review+
I checked that patch in.  If that's all for the tree, I guess we can resolve
this; otherwise, I'm looking forward to more patches.  ;)
Patch for stuff in mozilla/browser and mozilla/toolkit coming up...
Attachment #149126 - Flags: superreview?(bzbarsky)
Attachment #149126 - Flags: review?(bzbarsky)
Comment on attachment 149126 [details] [diff] [review]
patch for firefox stuff
[Checkin: Comment 15]

This needs reviews from firefox people....
Attachment #149126 - Flags: superreview?(bzbarsky)
Attachment #149126 - Flags: superreview?(bugs)
Attachment #149126 - Flags: review?(mconnor)
Attachment #149126 - Flags: review?(bzbarsky)
Comment on attachment 149126 [details] [diff] [review]
patch for firefox stuff
[Checkin: Comment 15]

>RCS file: /cvsroot/mozilla/browser/config/mozconfig,v
>retrieving revision 1.8
>diff -u -8 -r1.8 mozconfig
>--- browser/config/mozconfig	7 May 2004 01:09:08 -0000	1.8
>+++ browser/config/mozconfig	23 May 2004 02:40:26 -0000
>@@ -1,14 +1,15 @@
>-
>+mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/firegtk2
>+ac_add_options --enable-default-toolkit=gtk2 

This must have been included by mistake. :)

r=mconnor@myrealbox.com excluding the bit above (no SR needed on this)	

will check this in today, thanks for the patch!
Attachment #149126 - Flags: superreview?(bugs)
Attachment #149126 - Flags: review?(mconnor)
Attachment #149126 - Flags: review+
(In reply to comment #10)
> This needs reviews from firefox people....

No it didn't, really.  See http://www.mozilla.org/projects/firefox/review.html

  For codebase-wide simple, repetitive changes [...] review from a Firefox Peer
  is not required as long as the patch as a whole has review.
Except that wasn't a codebase-wide change -- it was a firefox-specific change.
I'll note that patch != change.  The change was codebase-wide, but that change
was split into multiple patches, one of which was a firefox-specific patch.  It
has reviews though which is the important thing.
checked in on aviary and trunk

this is one of those hair-splitting things, if it was part of the original
patch, or even submitted separately but bz gave r+sr to the overall change, I
don't think we'd bat an eye at it.

leaving open in case there's more work on this bug to be done
Product: Browser → Seamonkey
I would be willing to work on this, if someone wants to email me a little more info.

jason at mqtclct . com
i'll put it here so other people can read it too if interested.

Basically all
nsString()
nsAutoString()
NS_LITERAL_STRING("")

should be replaced by

EmptyString()

(note that this is only without arguments. nsString(...) should not be replaced)

And
nsCString()
nsCAutoString()
NS_LITERAL_CSTRING("")

should be replaced by EmptyCString()

Hopefully there shouldn't be that many left. And if there are none then this bug
should be closed.
Attached patch Replaces some nsString()s (obsolete) — Splinter Review
This should replace all nsString()-occurences I found by EmptyString().
There are also a couple of new nsString() and ~nsString() calls, but unlike
nsString() EmptyString() is no class, so they can't be replaced (and don't
compile).
Now I'm going to look for the others (nsCString(), nsAutoString(), ...)
Attachment #169624 - Flags: review?
Comment on attachment 169624 [details] [diff] [review]
Replaces some nsString()s

- In xpcom/build/dlldeps.cpp

@@ -128,3 +128,3 @@ void XXXNeverCalled()
     NS_QuickSort(nsnull, 0, 0, nsnull, nsnull);
-    nsString();
+    EmptyString();
     NS_ProxyRelease(nsnull, nsnull, PR_FALSE);

This one is an odd case that we shouldn't change, this empty string constructor
call is here to ensure that the string constructor is properly exported from
this library, so of all these in our codebase, this is the one we want to keep.

r=jst

PS: When requesting reviews, type the email address of the person you want to
review this change (me for instance in this case) in the requestee field, or
it's likely that you won't get a review...
Attachment #169624 - Flags: superreview?(peterv)
Attachment #169624 - Flags: review?
Attachment #169624 - Flags: review+
This patch (including the changes of the previous patch) replaces all
occurences of nsC?String(), nsAutoC?String() and NS_LITERAL_C?STRING("") by
EmptyC?String().
Except constructor/destructor calls of the classes nsC?String and
nsAutoC?String.
Of course I can't promise the patch works, but it compiles and this attachment
is submitted using Mozilla with the patch applied...
Attachment #169624 - Attachment is obsolete: true
Attachment #169627 - Flags: review?
(In reply to comment #19)
> (From update of attachment 169624 [details] [diff] [review] [edit])
> - In xpcom/build/dlldeps.cpp
> 
> @@ -128,3 +128,3 @@ void XXXNeverCalled()
>      NS_QuickSort(nsnull, 0, 0, nsnull, nsnull);
> -    nsString();
> +    EmptyString();
>      NS_ProxyRelease(nsnull, nsnull, PR_FALSE);
> 
> This one is an odd case that we shouldn't change, this empty string constructor
> call is here to ensure that the string constructor is properly exported from
> this library, so of all these in our codebase, this is the one we want to keep.
> 
> r=jst
> 
> PS: When requesting reviews, type the email address of the person you want to
> review this change (me for instance in this case) in the requestee field, or
> it's likely that you won't get a review...
> 

I'm sorry for next patch (includes dlldeps.cpp change), I haven't noticed your
answer before submitting...
Attached patch Removes dlldeps.cpp change. (obsolete) — Splinter Review
Now the same patch without this dlldeps.cpp-change (of course, it's not much
work to derive this one of the last...). There are still other changes on
dlldeps.cpp, but inside other calls. So I think they can be changed.
Attachment #169627 - Attachment is obsolete: true
Attachment #169628 - Flags: review?(jst)
Attachment #169627 - Flags: review?
Comment on attachment 169628 [details] [diff] [review]
Removes dlldeps.cpp change.

Looks good to me, r=jst
Attachment #169628 - Flags: review?(jst) → review+
Actually, is the change to xpcom/tests/StringFactoringTests/profile_main.cpp
correct?  It looks like it's no longer testing nsCString construction....  Then
again, is that file even used?
I don't know what exactly xpcom/tests/StringFactoringTests/profile_main.cpp is
for, but if you want to know if the nsString::constructor is visible or works,
it must not be changed. If this is so, I'll undo the changes.
Yeah, I suspect that's what that file is for.
Attachment #169628 - Attachment is obsolete: true
Attachment #169929 - Flags: review?(bzbarsky)
Comment on attachment 169929 [details] [diff] [review]
Removes testfile changes
[Checkin: Comment 29]

sr=bzbarsky (assuming the r=jst, since the patch didn't really change since
then).
Attachment #169929 - Flags: review?(bzbarsky) → superreview+
Attachment #169624 - Flags: superreview?(peterv) → superreview+
Attachment #169624 - Flags: superreview+
Comment on attachment 169929 [details] [diff] [review]
Removes testfile changes
[Checkin: Comment 29]

I just checked this patch in to trunk for 1.8a6.  If there's nothing left to do
here, the bug should probably be resolved fixed.
I don't think there's anything left to do (I replaced all hits except of
con-/destructor calls and calls inside of those test functions). Anyone else
should look at this and resolve the bug.
Sorry, I don't know why, but there are still some lxr hits left. I'll look at
them when I find time. 
Note that lxr doesn't update instantaneously, so you may be seeing hits you just
fixed with that patch.
lxr has updated.  This looks fixed.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
examining... patch to follow...
Attached patch latest (obsolete) — Splinter Review
Takes care of:

- all NS_LITERAL_STRING("")s
- all nsString()s that appeared to need changing

Does not take care of:

- NS_LITERAL_CSTRING("") in /Java
- nsCString in /Camino

Of course, if there is anything else that should be changed, that I have not
adressed here, note in comments and I'll take care of it ASAP.
Attachment #179813 - Flags: superreview?(bzbarsky)
Attachment #179813 - Flags: review?(bzbarsky)
Why not fix it all (i.e. why the "Does not take care of" list?)

Instead of doing |contextStr = NS_LITERAL_STRING("")| do |contextStr.Truncate()|
in libeditor.
Attached patch latest improved (obsolete) — Splinter Review
After checking out the needed files, I have made the appropriate changes in
them as well.  Therefore, this patch covers everything that I noticed. 

Also covers the change requested in comment 37.
Attachment #179813 - Attachment is obsolete: true
Attachment #179867 - Flags: superreview?(bzbarsky)
Attachment #179867 - Flags: review?(bzbarsky)
Comment on attachment 179867 [details] [diff] [review]
latest improved

>Index: camino/src/preferences/PreferenceManager.mm
>===================================================================
> 
>-  rv = NS_NewNativeLocalFile(nsCString(), PR_TRUE, getter_AddRefs(profDirFile));
>+  rv = NS_NewNativeLocalFile(EmptyString(), PR_TRUE, getter_AddRefs(profDirFile));
>   if (NS_SUCCEEDED(rv)) {
>     // profDirDesc is ASCII so no loss


EmptyString seems like wrong type, should be EmptyCString
Attached patch latest improved corrected (obsolete) — Splinter Review
Thank you for noticing the error.  My apologies for lack of attention to
detail.
Attachment #179867 - Attachment is obsolete: true
Attachment #179873 - Flags: superreview?(bzbarsky)
Attachment #179873 - Flags: review?(bzbarsky)
Attachment #179867 - Flags: superreview?(bzbarsky)
Attachment #179867 - Flags: review?(bzbarsky)
Attachment #179813 - Flags: superreview?(bzbarsky)
Attachment #179813 - Flags: review?(bzbarsky)
Comment on attachment 179873 [details] [diff] [review]
latest improved corrected

r+sr=bzbarsky; checkins need approval right now, so please remind me to check
this in once the tree reopens, ok?
Attachment #179873 - Flags: superreview?(bzbarsky)
Attachment #179873 - Flags: superreview+
Attachment #179873 - Flags: review?(bzbarsky)
Attachment #179873 - Flags: review+
Given how much longer the tree is likely to be closed, I'd just request approval
and find someone to check this in (I can't do the latter until July...)
Comment on attachment 179873 [details] [diff] [review]
latest improved corrected

Requesting approval.  Nice simple patch and all...
Attachment #179873 - Flags: approval1.8b4?
When this gets checked in, one of the semicolons here should be removed...

+++ editor/libeditor/html/nsHTMLDataTransfer.cpp	6 Apr 2005 17:02:05 -0000
+    contextStr.Truncate();;
Attachment #179873 - Flags: approval1.8b4? → approval1.8b4+
Comment on attachment 189470 [details] [diff] [review]
Same patch updated to tip
[Checkin: Comment 46]

Checked this in for 1.8b4. Can this be re-marked fixed?
Attachment #189470 - Attachment is obsolete: true
How about the places that do "nsAutoString empty;" for an empty string?  There's
about 15 of those.
Fun.  Post patches!
Attached patch nsAutoString -> EmptyString() (obsolete) — Splinter Review
In content/xml/document/src/nsXMLDocument.cpp, why does it Truncate the
nsAutoString?

<http://lxr.mozilla.org/mozilla/source/extensions/xforms/nsXFormsXPathParser.cpp#564>

Here, I tried to use EmptyString(), but that gave me some compile error. Silly
as it might sound, I don't know what error. My terminal is not wide enough to
show the errors. Anyone know how to fix that in any way? Some option to gmake
that makes it wrap the errors instead of concatenating them?
Attachment #195565 - Flags: review?(bzbarsky)
> In content/xml/document/src/nsXMLDocument.cpp, why does it Truncate the
> nsAutoString?

I don't think we want to think about it... ;)  There's no good reason for it.

> Here, I tried to use EmptyString(), but that gave me some compile error.

The method it's being passed to should take a const string, but wants a
non-const one.  Just change the method signature?

Comment on attachment 195565 [details] [diff] [review]
nsAutoString -> EmptyString()

>Index: content/xul/content/src/nsXULElement.cpp
>+                if (NS_FAILED(ret = listenerManager->CreateEvent(aPresContext, aEvent, EmptyString(), aDOMEvent))) {

Wrap to 80 chars, please.

>+            if (NS_FAILED(ret = listenerManager->CreateEvent(aPresContext, aEvent, EmptyString(), aDOMEvent)))

Same.

r=bzbarsky with that fixed.
Attachment #195565 - Flags: review?(bzbarsky) → review+
I changed nsXFormsXPathParser.cpp::XPathCompilerException to take a const,
recompiled and ran some XForms W3C tests.

Who should sr?
Attachment #195565 - Attachment is obsolete: true
Attachment #195692 - Flags: review+
Gah! Forgot a file in the patch. Sorry about the spam.
Attachment #195693 - Attachment is obsolete: true
Attachment #195694 - Flags: review?(bzbarsky)
Attachment #195693 - Flags: review?(bzbarsky)
(In reply to comment #53)
> And now .. this one has me puzzled:
> <http://lxr.mozilla.org/seamonkey/source/mailnews/base/src/nsMsgDBView.cpp#546>.

That is a (wrong) attempt to do the same as ToNewUnicode(EmptyString()).
Comment on attachment 195692 [details] [diff] [review]
nsAutoString -> EmptyString() v0.2
[Checkin: Comment 56]

Checked this in.
Comment on attachment 195694 [details] [diff] [review]
nsString() -> EmptyString() (+1 small literal) v0.2

Please find a different reviewer; I'm not a peer for the vast majority of this code, and even if I were I wouldn't get to this till at least January.
Attachment #195694 - Flags: review?(bzbarsky)
Comment on attachment 195694 [details] [diff] [review]
nsString() -> EmptyString() (+1 small literal) v0.2

I'm frankly not sure who to request from here. Maybe jag for general string use?  If not, maybe brendan for catch-all...
Attachment #195694 - Flags: review?(jag)
Hmmm. This "good first bug" is in search of a reviewer since Dec. 2005. Takers, anyone?
ugh, indeed. Does the patch still apply? My guess is that it needs to be updated to trunk. Also, right now would not be the right time to land it since we're so close to release.
Well, the author of the latest patches is "not reading bugmail", so I'm clearing the assignee field so as not to deter new takers. The first task will, I suppose, be to un-bitrot the patch.
Assignee: charles.fenwick → nobody
Status: REOPENED → NEW
I bet this could be trivially done now with our static analysis tools. No need to do this by hand any more...
(In reply to comment #63)
> I bet this could be trivially done now with our static analysis tools. No need
> to do this by hand any more...

It would be interesting to see if someone who is not me or Taras can use Dehydra [1] on this. Automatically generating patches is harder, but a script to find the sites to be changed should not be too hard. After that, someone could make the edits manually if there are few enough, or get in touch with us on creating a patch generator. The checker/patcher could be kept around to take care of any of the old patterns that sneak back in.

The idea would be to use Dehydra to walk over all the function calls (including methods and constructors) in the code, find the call sites for the selected constructors, and print the locations. Dehydra has a standard 'iter' function (see system.js) for iterating over statements. I also have an JS 1.8 iterator function that I find personally easier to use, let me know if anyone wants it.

[1] http://developer.mozilla.org/en/docs/Dehydra_GCC
<http://mxr.mozilla.org/comm-central/search?string=NS_LITERAL_STRING%28%22%22%29>
{{
NS_LITERAL_STRING("")

Found 7 matching lines in 6 files
}}
referring to the above comment, now it says no matching results.
how about derived classes of nsString or nsAutoString?
Assignee: nobody → edmund
- Replacing NS_LITERAL_STRING("") in the mailnews repository.
Attachment #428889 - Flags: review?
Attachment #428889 - Flags: review? → review?(bzbarsky)
- NS_LITERAL_STRING("") -> EmptyString()
Attachment #428890 - Flags: review?(bzbarsky)
Attachment #195692 - Attachment description: nsAutoString -> EmptyString() v0.2 → nsAutoString -> EmptyString() v0.2 [Checkin: Comment 56]
Attachment #149095 - Attachment description: latest and greatest → latest and greatest [Checkin: Comment 7]
Severity: normal → trivial
Product: SeaMonkey → Core
QA Contact: general → general
Attachment #149126 - Attachment description: patch for firefox stuff → patch for firefox stuff [Checkin: Comment 15]
Attachment #169929 - Attachment description: Removes testfile changes. → Removes testfile changes [Checkin: Comment 29]
Attachment #189470 - Attachment description: Same patch updated to tip → Same patch updated to tip [Checkin: Comment 46]
Attachment #189470 - Attachment is obsolete: false
Status: NEW → ASSIGNED
Attachment #432299 - Flags: review? → review?(jonas)
(In reply to comment #71)
> You attached the same patch twice...
> 
> See at least:
> http://mxr.mozilla.org/comm-central/search?string=String+empty&case=1&find=%5C.cpp

Nix.  Sorry.  will replace it with the right one.
Attachment #432299 - Attachment is obsolete: true
Attachment #432501 - Flags: review?(jonas)
Attachment #432299 - Flags: review?(jonas)
Attachment #428889 - Flags: superreview?(bzbarsky)
Attachment #428890 - Flags: superreview?(bzbarsky)
Attachment #432300 - Flags: superreview?(bzbarsky)
Attachment #432501 - Flags: superreview?(bzbarsky)
Comment on attachment 428889 [details] [diff] [review]
Changing NS_LITERAL_STRING("") to EmptyString() [Checkin c#88]

sr=dbaron, although I don't think this actually needs sr
Attachment #428889 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 428890 [details] [diff] [review]
Replacing NS_LITERAL_STRING("") in the mozilla tree.

>-    wm->GetMostRecentWindow(NS_LITERAL_STRING("").get(), getter_AddRefs(window));
>+    wm->GetMostRecentWindow(EmptyString(), getter_AddRefs(window));

Maybe leave the .get() unless you're confident it's not needed across all relevant platforms?

sr=dbaron, although I don't think this actually requires sr
Attachment #428890 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 432300 [details] [diff] [review]
Patch to remove incident of nsAutoString empty.

sr=dbaron, although I don't think this needs superreview
Attachment #432300 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 432501 [details] [diff] [review]
Removed declaration of nsAutoString empty. Replaced all 'empty' occurrences with EmptyString()

>-            nsAutoString empty;
>-            m_compFields->SetTo(empty);
>-            m_compFields->SetCc(empty);
>-            m_compFields->SetBcc(empty);
>-            m_compFields->SetNewsgroups(empty);
>-            m_compFields->SetFollowupTo(empty);
>+            m_compFields->SetTo(EmptyString());
>+            m_compFields->SetCc(EmptyString());
>+            m_compFields->SetBcc(EmptyString());
>+            m_compFields->SetNewsgroups(EmptyString());
>+            m_compFields->SetFollowupTo(EmptyString());

It might be better to do:
  const nsAString& empty = EmptyString();
and then leave the other lines as-is.  But sr=dbaron either way, although I don't think this requires sr.
Attachment #432501 - Flags: superreview?(bzbarsky) → superreview+
Could you add the 'checkin-needed' keyword after revising the patches appropriately?
Attachment #432501 - Attachment is obsolete: true
Attachment #428890 - Attachment is obsolete: true
(In reply to comment #75)
>(From update of attachment 428890 [details] [diff] [review])
>>-    wm->GetMostRecentWindow(NS_LITERAL_STRING("").get(), getter_AddRefs(window));
>>+    wm->GetMostRecentWindow(EmptyString(), getter_AddRefs(window));
>Maybe leave the .get() unless you're confident it's not needed across all
>relevant platforms?
FYI GetMostRecentWindow happens to be nsnull-safe anyway.
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/9fbcdeef28c0
Status: ASSIGNED → RESOLVED
Closed: 15 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug] → c-n 428889 & 441232 to comm-central [good first bug]
Target Milestone: --- → mozilla1.9.3a5
(In reply to comment #82)
> (In reply to comment #75)
> >(From update of attachment 428890 [details] [diff] [review] [details])
> >>-    wm->GetMostRecentWindow(NS_LITERAL_STRING("").get(), getter_AddRefs(window));
> >>+    wm->GetMostRecentWindow(EmptyString(), getter_AddRefs(window));
> >Maybe leave the .get() unless you're confident it's not needed across all
> >relevant platforms?
> FYI GetMostRecentWindow happens to be nsnull-safe anyway.

This broke the Maemo mobile builds. Working on a patch to add ".get()" back.
This patch adds the ".get()" back into the code. I am building locally to verify this is the only part that breaks Maemo (it should be).
Attachment #445973 - Flags: review?(bzbarsky)
Attachment #445973 - Flags: review?(bzbarsky) → review+
(In reply to comment #86)
> pushed maemo bustage fix:
> http://hg.mozilla.org/mozilla-central/rev/2ea3130f9f7b

My apologies for the bustage.  Will be more careful next time.
Comment on attachment 428889 [details] [diff] [review]
Changing NS_LITERAL_STRING("") to EmptyString() [Checkin c#88]

Pushed as: http://hg.mozilla.org/comm-central/rev/2c9ffd5cdc8a
Attachment #428889 - Attachment description: Changing NS_LITERAL_STRING("") to EmptyString() → Changing NS_LITERAL_STRING("") to EmptyString() [Checkin c#88]
Comment on attachment 441232 [details] [diff] [review]
Defined empty to be EmptyString()

Pushed as: http://hg.mozilla.org/comm-central/rev/37547c236d72
Attachment #441232 - Attachment description: Defined empty to be EmptyString() → Defined empty to be EmptyString() [Checkin c#89]
Keywords: checkin-needed
Whiteboard: c-n 428889 & 441232 to comm-central [good first bug]
Whiteboard: [good first bug]
Comment on attachment 441232 [details] [diff] [review]
Defined empty to be EmptyString()

Backed this out in http://hg.mozilla.org/comm-central/rev/8d290e5c854d

Due to a build error:
/builds/slave/macosx-comm-central-check/build/mailnews/compose/src/nsMsgCompose.cpp:2108: error: invalid initialization of reference of type 'const nsAutoString&' from expression of type 'const nsString'
Attachment #441232 - Attachment description: Defined empty to be EmptyString() [Checkin c#89] → Defined empty to be EmptyString()
Attached patch nsMsgComposeSplinter Review
Rescued from attachment 195694 [details] [diff] [review] as well. This was the only other hunk that still existed.
Attachment #195694 - Attachment is obsolete: true
Attachment #552626 - Flags: review?(jonathan.protzenko)
Attachment #195694 - Flags: review?(jag-mozilla)
Attachment #552623 - Flags: review?(ehsan) → review+
Attachment #552626 - Flags: review?(jonathan.protzenko) → review?(dbienvenu)
Comment on attachment 552626 [details] [diff] [review]
nsMsgCompose

this doesn't compile - but when protz makes that method scriptable, he could fix this.
Attachment #552626 - Flags: review?(dbienvenu) → review-
You need to log in before you can comment on or make changes to this bug.