1432519 - Make nsIURL attributes readonly

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P3)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=74e9afa76847a7193ba31c4b25ec5e11718bc570

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1432519 - Make nsIURL attributes readonly

Review commit: https://reviewboard.mozilla.org/r/214936/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214936/

Comment 3

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5064b9aa1377270124c1d0c896f7c0fe68f8e192

Comment 4

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4c46be86b276311c97be2b1d79afa3ed5ad3b46

Comment 5

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7915924e1b19c4f9b0ba4371cbf17fd99a0e8fa0

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8944773 [details]
Bug 1432519 - Make nsIURL attributes readonly

https://reviewboard.mozilla.org/r/214936/#review220750

::: commit-message-676a8:1
(Diff revision 1)
> +Bug 1432519 - Make nsIURI attributes readonly

Nit: nsIURL

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 8944773 [details]
Bug 1432519 - Make nsIURL attributes readonly

https://reviewboard.mozilla.org/r/214936/#review220886

::: commit-message-676a8:1
(Diff revision 1)
> +Bug 1432519 - Make nsIURI attributes readonly

actually quite important nit!! :)

::: netwerk/base/nsIURL.idl:65
(Diff revision 1)
>       * Some characters may be escaped.
>       */
> -    attribute AUTF8String fileName;
> +    readonly attribute AUTF8String fileName;
>  
> +    [noscript,notxpcom,nostdcall]
> +    nsresult setFileNameInternal(in ACString aFileName);

wasn't the whole purpose of this enterprise to remove public setters on all UR* interfaces?

Comment 8

[Valentin Gosu [:valentin] (he/him)]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> ::: netwerk/base/nsIURL.idl:65
> > +    [noscript,notxpcom,nostdcall]
> > +    nsresult setFileNameInternal(in ACString aFileName);
> 
> wasn't the whole purpose of this enterprise to remove public setters on all
> UR* interfaces?

You're absolutely right. Initially I thought we might need these to be on the URI to use in GTests.
We actually don't need them. I'll make them private to the implementations.

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8944773 [details]
Bug 1432519 - Make nsIURL attributes readonly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214936/diff/1-2/

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Ignore the nsIFileURL changes in the latest diff. I rebased this patch on top of bug 1432602.

Comment 11

[Honza Bambas (:mayhemer)]

Comment on attachment 8944773 [details]
Bug 1432519 - Make nsIURL attributes readonly

https://reviewboard.mozilla.org/r/214936/#review222038

::: netwerk/base/nsStandardURL.h:457
(Diff revisions 1 - 2)
> +
> +            nsresult rv = uri->SetFile(aFile);
> +            if (NS_FAILED(rv)) {
> +                return rv;
> +            }
> +            BaseURIMutator<T>::mURI = uri;

nit: swap?

Comment 12

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8944773 [details]
Bug 1432519 - Make nsIURL attributes readonly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214936/diff/2-3/

Comment 13

[Valentin Gosu [:valentin] (he/him)]

I plan to land this bug next week.
mailnews needs to implement changes similar to bug 1432320 by then.

Comment 14

[Jorg K (CEST = GMT+2)]

Thanks, I filed bug 1434687 for that. If I find time, I might prepare a patch for C-C by applying the patch here locally. Or I'll just deal with the bustage which I don't expect to be large. I haven't taken a good look at how much JS code is affected.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/178575b934ea
Make nsIURL attributes readonly r=mayhemer

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/178575b934ea