751785 - Make Selection.toString() API use DOMString instead of wstring

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Mats Palmgren (inactive)]

Make Selection.toString() API use DOMString instead of wchar,
per bug 748961 comment 11.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Need to change the declarations in

http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsISelection.idl#165
http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsISelectionPrivate.idl#88

to use "DOMString" instead of "wstring", and the implementations in

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp#1313

The PRUnichar** arguments should become nsAString& in nsTypedSelection::ToString and nsTypedSelection::ToStringWithFormat.

Comment 2

[Chance Zibolski]

I'm looking at this bug as an introduction to the mozilla bug tracker and other things, but the PRUnichar** arguments are pointers to that type if im right, so changing them to nsAString& does what? Is that a type?

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

nsAString& is a writable abstract string type; see <https://developer.mozilla.org/En/Mozilla_internal_string_guide> for more information about it.

Comment 4

[Jignesh Kakadiya [:jhk]]

Attachment: Patch(v1)

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 631322 [details] [diff] [review]
Patch(v1)

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

This looks great; just a few comments. Can you fix those and ask :smaug to review?

::: content/base/public/nsISelection.idl
@@ +134,5 @@
>  
>      /**
>       * Returns the whole selection into a plain text string.
>       */
> +    DOMString toString();

Update the uuid.

::: layout/generic/nsSelection.cpp
@@ +1094,5 @@
>    // null if the Selection has been disconnected (the shell is Destroyed).
>    nsCOMPtr<nsIPresShell> shell =
>      mFrameSelection ? mFrameSelection->GetShell() : nsnull;
>    if (!shell) {
> +    aReturn = ToNewUnicode(EmptyString());

This should just be 'aReturn.Truncate();' now.

@@ +1145,1 @@
>    return rv;

And these four lines can be 'return encoder->EncodeToString(aReturn);'

Comment 6

[Jignesh Kakadiya [:jhk]]

Attachment: Patch(v2)

Comment 7

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34117bad5665

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34117bad5665

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Ms2ger pushed a follow-up to comm-central fix bustage there as well.
https://hg.mozilla.org/comm-central/rev/1608d7734acb