1434163 - Make all nsIURI attributes readonly

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Comment 1

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

Attachment: Bug 1434163 - Make all nsIURI attributes readonly

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

Comment 2

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

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

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8946535 [details]
Bug 1434163 - Make all nsIURI attributes readonly

https://reviewboard.mozilla.org/r/216554/#review222432

::: netwerk/base/nsIURI.idl:140
(Diff revision 1)
> -    attribute AUTF8String password;
> +    readonly attribute AUTF8String password;
>  
>      /**
>       * The host:port (or simply the host, if port == -1).
>       *
>       * If this attribute is set to a value that only has a host part, the port

it can no longer be set, this comment should probably be on the mutator?

::: netwerk/test/gtest/TestStandardURL.cpp:13
(Diff revision 1)
>  #include "nsPrintfCString.h"
>  #include "nsComponentManagerUtils.h"
>  #include "nsIURIMutator.h"
> +#define protected public
> +#include "../../base/nsStandardURL.h"
> +using mozilla::net::nsStandardURL;

wow!  that's a *HACK* :D

you should at least #undef it :)))

can't this somehow be achieved by deriving from nsStandardURL and 'public: using blabla' ?

nsStandardURL is not marked final (but maybe should be....)

other trick could be to have a predeclared friend class that will be implemented here

OTOH, it's just a test and if it builds on all platforms then why not take this...

Comment 4

[Jorg K (CEST = GMT+2)]

Hmm, a few methods are become private here, like SetSpecInternal().

Comment 5

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

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

Comment 6

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

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

Comment 7

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

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

Comment 8

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

Comment on attachment 8946535 [details]
Bug 1434163 - Make all nsIURI attributes readonly

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

Comment 9

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

Comment on attachment 8946535 [details]
Bug 1434163 - Make all nsIURI attributes readonly

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

Comment 10

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

Comment on attachment 8946535 [details]
Bug 1434163 - Make all nsIURI attributes readonly

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

Comment 11

[Jorg K (CEST = GMT+2)]

Any ETA for landing this?

Comment 12

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

I was thinking of landing it today or tomorrow, after a final try run.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74fc20523198
Make all nsIURI attributes readonly r=mayhemer

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/74fc20523198

Comment 15

[Kent James (:rkent)]

We were sort of hoping this would have qualified as a "risky patch or a large patch" that would be subject to the March 1 code freeze for Firefox 60 that was announced on m.d.platform. I guess not :(

Comment 16

[Jorg K (CEST = GMT+2)]

The risky one was bug 1433958 which landed on 27th Feb. ;-)