Last Comment Bug 751785 - Make Selection.toString() API use DOMString instead of wstring
: Make Selection.toString() API use DOMString instead of wstring
Status: RESOLVED FIXED
[good first bug][mentor=Ms2ger][lang=...
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: mozilla16
Assigned To: Jignesh Kakadiya [:jhk]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 17:29 PDT by Mats Palmgren (:mats)
Modified: 2012-06-10 10:39 PDT (History)
5 users (show)
dzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch(v1) (4.50 KB, patch)
2012-06-08 02:43 PDT, Jignesh Kakadiya [:jhk]
Ms2ger: feedback+
Details | Diff | Splinter Review
Patch(v2) (5.67 KB, patch)
2012-06-08 03:30 PDT, Jignesh Kakadiya [:jhk]
bugs: review+
Details | Diff | Splinter Review

Description Mats Palmgren (:mats) 2012-05-03 17:29:12 PDT
Make Selection.toString() API use DOMString instead of wchar,
per bug 748961 comment 11.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2012-05-04 02:34:09 PDT
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 2012-05-07 18:14:32 PDT
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 (⌚ UTC+1/+2) 2012-05-09 10:38:28 PDT
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] 2012-06-08 02:43:11 PDT
Created attachment 631322 [details] [diff] [review]
Patch(v1)
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2012-06-08 02:46:41 PDT
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] 2012-06-08 03:30:05 PDT
Created attachment 631333 [details] [diff] [review]
Patch(v2)
Comment 7 David Zbarsky (:dzbarsky) 2012-06-09 15:21:25 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/34117bad5665
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-06-09 19:48:28 PDT
https://hg.mozilla.org/mozilla-central/rev/34117bad5665
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-06-10 10:39:01 PDT
Ms2ger pushed a follow-up to comm-central fix bustage there as well.
https://hg.mozilla.org/comm-central/rev/1608d7734acb

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