88413 - Remove GetUnicode() from nsString

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[jag (Peter Annema)]

Remove GetUnicode from nsString and replace all call-sites with get().

Patch coming up.

Comment 1

[jag (Peter Annema)]

Attachment: [patch] mechanical change from all nsString::GetUnicode() to nsString::get()

Comment 2

[Scott Collins]

rs=scc

Comment 3

[jag (Peter Annema)]

Attachment: [patch] Part two: actually remove GetUnicode from nsString2.h

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If you want to check this in this weekend you might want to find someone
to do the security partition stuff now.  However, you may want to wait a
bit to check in the second patch.  (You also need to look at the
commercial tree.)

(nsWebShell.cpp)
> @@ -824,7 +824,7 @@
>                            nsnull,             // No onwer
>                            PR_TRUE,            // Inherit owner from document
>                            PR_FALSE,           // Do not stop active document
> -                          target.GetUnicode(),// Window target
> +                          target.get(),// Window target
>                            aPostDataStream,    // Post data stream
>                            aHeadersDataStream, // Headers stream
>                            LOAD_LINK,          // Load type

Indentation.


(nsHTMLEditor.cpp)
> @@ -2961,7 +2961,7 @@
>        nsAutoString href;
>        res = anchor->GetHref(href);
>        if (NS_FAILED(res)) return res;
> -      if (href.GetUnicode() && href.Length() > 0)
> +      if (href.get() && href.Length() > 0)
>        {
>          nsAutoEditBatch beginBatching(this);
>          nsString attribute(NS_LITERAL_STRING("href"));

This condition should be |if (!href.IsEmpty())|

> Index: embedding/tests/qa_mfcembed/BV-cpp.txt
> Index: embedding/tests/qa_mfcembed/BV2-cpp.txt

Do you want to be changing these files?

> Index: string/doc/string-guide.html

It seems a little odd to change quoted text, but probably better than
the confusion that could otherwise result.


For the second patch, I think nsString::get() could probably be inline
and that check could be an assertion, although this may not be the best
time to make that change and we may not care.

r=dbaron on both patches.

Comment 5

[jag (Peter Annema)]

I opted not to change those two .txt files nor the string-guide.html.

scc, could you update this part of string-guide.html in a suitable manner next
time you add to it? Thanks.

First patch almost completely checked in, will need help with security/* and
Netscape commercial.

Comment 6

[Scott Collins]

unfortunately, |get()| has to be |virtual| for the moment, see

  http://lxr.mozilla.org/mozilla/source/string/public/nsAFlatString.h#37

an |inline| factored |get| for all single-fragment strings requires working
implementations of |GetBufferHandle|, which obsolete |nsString| cannot provide.
 These days, an |nsString| can't end up pointing to a |eOneByte| buffer, so that
test has become meaningless, and might as well be

  return mUStr;

If you're paranoid (as perhaps you should be) you could recapitulate the old
test with an |NS_ASSERTION|.

  NS_ASSERTION(mCharSize==eOneByte, "malformed |nsString|!");
  return mUStr;

other than that, sr=scc

Comment 7

[jag (Peter Annema)]

|GetUnicode()| is dead, long live |get()|.

Comment 8

[David Epstein]

just to point out the qa_mfcEmbed is now obsolete. Replaced by testEmbed in
/mozilla/embedding/qa/testembed.