1256030 - Remove deprecated functions calls from NetUtil.jsm

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1256030_remove_deprecated_functions_from_netutiljsm.patch

Hey Pat, sorry I forgot that the first time around within Bug 1254752, but we also have to update NetUtil.jsm. There is also test_NetUtil.js which provides pretty good testcoverage for the functionality provided within NetUtil.jsm - all tests pass.

Btw, we should also make sure to throw an exception if the arguemnts.length != 1 so we catch callsites which still use the old API right away.

Thanks!

Comment 2

[arni2033]

Will it fix Pocket?

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to arni2033 from comment #2)
> Will it fix Pocket?

Oh, pocket broke as well? I thought I fixed it with [1]. Apparently not. I'll have a look.

[1] https://hg.mozilla.org/mozilla-central/rev/0b1dbcc5db72

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)
> (In reply to arni2033 from comment #2)
> > Will it fix Pocket?

I filed Bug 1256050 and will fix Pocket there.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Pat, given comment [1] the change in that patch is probably not completely right either. What do you suggest we do for example for the case:
> NetUtil.newChannel(file);

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1254752#c25

Comment 6

[Diego Casorran [:diegocr]]

I guess this could be made working on a backward-compatible way by picking the triggeringPrincipal/securityFlags from the file/uri's protocol, and contentPolicyType from the extension?

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Jorge, because of the update within Bug 1254752, we also have to update NetUtil.newChannel() because ioservice.newChannelFromURI() was removed within Bug 1254752. This change will cause some addons to experience problems and those addons need to be updated.

I will make sure that we update all of our devdocs to reflect the necessary changes. Anything else I need to be aware of?

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1256030_remove_deprecated_functions_from_netutiljsm.patch

Pat, I discussed that briefly with Jonas and we both agree we should provide a way to also allow NetUtil.newChannel() to be called for loading an nsIFile. Hence I updated the patch to include that.

For addons the new API call would look like this:
> NetUtil.newChannel({uri: file, loadUsingSystemPrincipal: true});

Can we all agree on that?

Comment 9

[Patrick McManus [:mcmanus]]

Comment on attachment 8730351 [details] [diff] [review]
bug_1256030_remove_deprecated_functions_from_netutiljsm.patch

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

makes sense to me

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Patrick McManus [:mcmanus] from comment #9)
> makes sense to me

thanks Pat!

Comment 11

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)
> I will make sure that we update all of our devdocs to reflect the necessary
> changes. Anything else I need to be aware of?

That should be enough, thanks.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/043d7553482f

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/043d7553482f

Comment 14

[Christoph Kerschbaumer [:ckerschb]]

Please note that we are reverting changes within Bug 1257339 and restoring the deprecated NetUtil.newChannel() API for now. It's still deprecated and shouldn't be used anymore. However, addons have the desire to still use that API. So now, as a first step we are going to log a deprecation warning to the console.