Remove `wstring` attributes

RESOLVED FIXED in Firefox 64

Status

()

enhancement
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(8 attachments)

It's a step towards removing `wstring` altogether.
Component: General → XPCOM
This patch also removes the setTitle() method and makes `title` non-readonly.
Attachment #9006770 - Flags: review?(nika)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Attachment #9006774 - Flags: review?(valentin.gosu) → review+
Comment on attachment 9006771 [details] [diff] [review]
Change nsIDirIndex's wstring/string attributes to AString/ACString

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

::: netwerk/streamconv/nsIDirIndex.idl
@@ +54,5 @@
>      /**
>       * A description for the filename, which should be
>       * displayed by a viewer
>       */
> +    attribute AString description;

From what I can tell, there are no calls to Get/SetDescription outside the index parser, so let's make this an ACString if possible. That would simplify thing.
Attachment #9006771 - Flags: review?(valentin.gosu) → review+
Attachment #9006773 - Flags: review?(nfroyd) → review+
Attachment #9006770 - Flags: review?(nika) → review+
Attachment #9006775 - Flags: review?(nika) → review+
Attachment #9006772 - Flags: review?(erahm) → review+
Comment on attachment 9006776 [details] [diff] [review]
Change some nsIUserInfo attrributes arguments string/wsstring to ACString/AString

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

Thanks for the cleanup, just a few minor notes.

::: toolkit/components/startup/nsUserInfoUnix.cpp
@@ +134,3 @@
>      nsresult rv;
>  
>      nsAutoCString emailAddress;

This can go now.

@@ +147,4 @@
>          return NS_ERROR_FAILURE;
>      }
>  
> +    aEmailAddress = username.get();

Can you change this to |aEmailAddress = username + "@" + domain|, it's more straightforward and efficient. That might need to be NS_CSTRING_LITERAL("@") though.

::: toolkit/components/startup/nsUserInfoWin.cpp
@@ +33,5 @@
>    DWORD size = mozilla::ArrayLength(username);
>    if (!GetUserNameW(username, &size))
>      return NS_ERROR_FAILURE;
>  
> +  CopyUTF16toUTF8(nsDependentString(username), aUsername);

This could just be `MakeSpan(username, size - 1)` removing an intermediate. It probably doesn't matter that much.

@@ +44,5 @@
>    wchar_t fullName[512];
>    DWORD size = mozilla::ArrayLength(fullName);
>  
>    if (GetUserNameExW(NameDisplay, fullName, &size)) {
> +    aFullname.Assign(fullName);

Old nit, but we know the size here.

@@ +98,4 @@
>    }
>  
> +  CopyUTF16toUTF8(
> +    nsDependentString(

This can just be MakeStringSpan, but it probably doesn't matter.

@@ +118,5 @@
>      return getUsernameError == ERROR_NONE_MAPPED ?
>             NS_ERROR_NOT_AVAILABLE : NS_ERROR_FAILURE;
>    }
>  
> +  CopyUTF16toUTF8(nsDependentString(emailAddress), aEmailAddress);

Same here.

::: toolkit/components/startup/public/nsIUserInfo.idl
@@ +14,2 @@
>  
> +    readonly attribute ACString emailAddress;

Is this for consistency with the `wstring` change, or do we want to cleanup all `string` instances as well?
Attachment #9006776 - Flags: review?(erahm) → review+
> Is this for consistency with the `wstring` change, or do we want to cleanup all `string` instances as well?

A bit of both. This bug is mostly about `wstring`, but I changed a few `string` cases that were adjacent and similar to `wstring` instances.
(In reply to Valentin Gosu [:valentin] from comment #8)
> Comment on attachment 9006771 [details] [diff] [review]
> Change nsIDirIndex's wstring/string attributes to AString/ACString
> 
> Review of attachment 9006771 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/streamconv/nsIDirIndex.idl
> @@ +54,5 @@
> >      /**
> >       * A description for the filename, which should be
> >       * displayed by a viewer
> >       */
> > +    attribute AString description;
> 
> From what I can tell, there are no calls to Get/SetDescription outside the
> index parser, so let's make this an ACString if possible. That would
> simplify thing.

There's a call to gTextToSubURI->UnEscapeAndConvert() that produces one of the description values. That method is complicated enough that I don't want to much around with it. So I'll leave the patch unchanged.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1d9229766f4
Change nsISHEntry.title to an AString. r=nika
https://hg.mozilla.org/integration/mozilla-inbound/rev/067c003d0302
Change nsIDirIndex's wstring/string attributes to AString/ACString. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bcffbb1684c
Change some nsIUserInfo attrributes arguments string/wsstring to ACString/AString. r=erahm
https://hg.mozilla.org/integration/mozilla-inbound/rev/b82d2d4ad513
Change nsIConsoleMessage.message to an AString. r=erahm
https://hg.mozilla.org/integration/mozilla-inbound/rev/16060254450b
Change nsIWebBrowsing.searchString to an AString. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/d69f31a70f38
Change some nsICaptivePortalDetector method arguments from wstring to AString. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/4035a31a2996
Change some nsIWebNavigation method arguments from wstring to AString. r=nika
The method is gone because nsISHEntry.title is now mutable.
Attachment #9007100 - Flags: review?(frgrahl)
Comment on attachment 9007100 [details] [diff] [review]
Update comm-central for nsISHEntry.setTitle removal

Big thanks.
Attachment #9007100 - Flags: review?(frgrahl) → review+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/f3be60013adc
Update comm-central for nsISHEntry.setTitle removal. r=frg
You need to log in before you can comment on or make changes to this bug.