Stop implementing NPN_GetAuthenticationInfo

RESOLVED FIXED in Firefox 55

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: benjamin, Assigned: raoul_dcunha, Mentored)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Updated

2 years ago
Summary: Stop impelementing NPN_GetAuthenticationInfo → Stop implementing NPN_GetAuthenticationInfo
Comment hidden (offtopic)
Comment hidden (off-topic)
(Reporter)

Comment 3

2 years ago
Other than the specific test I mentioned, it would be good to run the major plugin tests: `mach mochitest dom/plugins`
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8856462 - Flags: review?(benjamin)
(Reporter)

Updated

2 years ago
Assignee: nobody → raoul_dcunha
(Reporter)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8856462 [details]
Bug 1352576 - Removed all instances of NPN_GetAuthenticationInfo from the codebase

https://reviewboard.mozilla.org/r/128420/#review130882

This is a great start! I've made one important note about not changing npfunctions.h because it's an external API layer (Flash depends on the current layout which we must not change). So I'll need to see an updated version with that fixed.

Once that's ready, do you have try server access, or shall I trigger a try build for you?

::: dom/plugins/base/npfunctions.h
(Diff revision 1)
>    NPN_EnumerateProcPtr enumerate;
>    NPN_PluginThreadAsyncCallProcPtr pluginthreadasynccall;
>    NPN_ConstructProcPtr construct;
>    NPN_GetValueForURLPtr getvalueforurl;
>    NPN_SetValueForURLPtr setvalueforurl;
> -  NPN_GetAuthenticationInfoPtr getauthenticationinfo;

NPAPI is a binary protocol, so we shouldn't change this file at all. Just keep this here and fill it in with nullptr in our implementation.

::: dom/plugins/base/nsNPAPIPlugin.cpp
(Diff revision 1)
>    _enumerate,
>    _pluginthreadasynccall,
>    _construct,
>    _getvalueforurl,
>    _setvalueforurl,
> -  _getauthenticationinfo,

nullptr here

::: dom/plugins/ipc/PluginModuleChild.cpp
(Diff revision 1)
>      mozilla::plugins::child::_enumerate,
>      mozilla::plugins::child::_pluginthreadasynccall,
>      mozilla::plugins::child::_construct,
>      mozilla::plugins::child::_getvalueforurl,
>      mozilla::plugins::child::_setvalueforurl,
> -    mozilla::plugins::child::_getauthenticationinfo,

And nullptr here
Attachment #8856462 - Flags: review?(benjamin) → review-
(Assignee)

Comment 7

2 years ago
mozreview-review-reply
Comment on attachment 8856462 [details]
Bug 1352576 - Removed all instances of NPN_GetAuthenticationInfo from the codebase

https://reviewboard.mozilla.org/r/128420/#review130882

Thanks for the review, Benjamin!
(Assignee)

Updated

2 years ago
Attachment #8857909 - Flags: review?(benjamin)
(Reporter)

Comment 8

2 years ago
Raoul, this looks like the correct change but it shouldn't be a new commit, rather a modification of the first. Can you fold these two commits together? You should be able to do this using the `hg histedit` command: https://www.mercurial-scm.org/wiki/HisteditExtension
Flags: needinfo?(raoul_dcunha)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8857909 - Attachment is obsolete: true
Attachment #8857909 - Flags: review?(benjamin)
(Assignee)

Comment 10

2 years ago
(In reply to Benjamin Smedberg [:bsmedberg] from comment #8)
> Raoul, this looks like the correct change but it shouldn't be a new commit,
> rather a modification of the first. Can you fold these two commits together?
> You should be able to do this using the `hg histedit` command:
> https://www.mercurial-scm.org/wiki/HisteditExtension

I've squashed the commits :). If you are happy with the change, can you trigger a try build for me as I don't have access to the try server
Flags: needinfo?(raoul_dcunha)
(Reporter)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8856462 [details]
Bug 1352576 - Removed all instances of NPN_GetAuthenticationInfo from the codebase

https://reviewboard.mozilla.org/r/128420/#review132682

Woot!
Attachment #8856462 - Flags: review?(benjamin) → review+

Comment 12

2 years ago
Pushed by bsmedberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f7780670900
Removed all instances of NPN_GetAuthenticationInfo from the codebase r=bsmedberg
https://hg.mozilla.org/mozilla-central/rev/5f7780670900
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.