Rename getNoAppCodebasePrincipal back to getCodebasePrincipal

RESOLVED FIXED in Firefox 17

Status

()

Core
Security: CAPS
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: akeybl, Assigned: sicking)

Tracking

({addon-compat})

Trunk
mozilla19
addon-compat
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17+ fixed, firefox18+ fixed, firefox19+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
(Jonas Sicking (:sicking) from bug 774585 comment #31)
> I'd be fine with naming back getNoAppCodebasePrincipal to
> getCodebasePrincipal if it's really needed. But if it's not affecting too
> many addons I'd prefer to keep things as-is.

Let's do this due to the add-on compatibility impact.
(Reporter)

Comment 1

5 years ago
Can we get this into tomorrow's beta to prevent any more add-on compat impact?
Keywords: addon-compat
Created attachment 676659 [details] [diff] [review]
Patch to fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 774585
User impact if declined: some addons will break. Unclear which
Testing completed (on m-c, etc.): None
Risk to taking this patch (and alternatives if risky): Very low to none. The patch adds a very simple function which just forwards to an existing call.
String or UUID changes made by this patch: nsIScriptSecurityManager iid is changed.
Attachment #676659 - Flags: review?(mounir)
Attachment #676659 - Flags: approval-mozilla-beta?
Attachment #676659 - Flags: approval-mozilla-aurora?
Comment on attachment 676659 [details] [diff] [review]
Patch to fix

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

r=me with the comments applied.

::: caps/idl/nsIScriptSecurityManager.idl
@@ +185,5 @@
> +    /**
> +     * DEPRECATED!
> +     * Legacy name for getNoAppCodebasePrincipal.
> +     */
> +    nsIPrincipal getCodebasePrincipal(in nsIURI uri);

Use [deprecated], please :)

https://developer.mozilla.org/en-US/docs/XPIDL

::: caps/src/nsScriptSecurityManager.cpp
@@ +2022,5 @@
>  NS_IMETHODIMP
> +nsScriptSecurityManager::GetCodebasePrincipal(nsIURI* aURI,
> +                                              nsIPrincipal** aPrincipal)
> +{
> +  return GetNoAppCodebasePrincipal(aURI, aPrincipal);

Stay consistent with other implementation and call "GetSimpleCodebasePrincipal".
Attachment #676659 - Flags: review?(mounir) → review+
> @@ +2022,5 @@
> >  NS_IMETHODIMP
> > +nsScriptSecurityManager::GetCodebasePrincipal(nsIURI* aURI,
> > +                                              nsIPrincipal** aPrincipal)
> > +{
> > +  return GetNoAppCodebasePrincipal(aURI, aPrincipal);
> 
> Stay consistent with other implementation and call
> "GetSimpleCodebasePrincipal".

That would be the wrong implementation as that would use the UNKNOWN_APP appid.
https://hg.mozilla.org/integration/mozilla-inbound/rev/434c6bdec0bb
Landed with a presumed a+ from lsblakk per his request on IRC to get these landed on aurora/beta ASAP for 17b4.

https://hg.mozilla.org/releases/mozilla-aurora/rev/54f51455ee2e
https://hg.mozilla.org/releases/mozilla-beta/rev/ae08e43155c3
status-firefox17: --- → fixed
status-firefox18: --- → fixed
status-firefox19: --- → fixed
Flags: in-testsuite?
Flags: in-testsuite? → in-testsuite+

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/434c6bdec0bb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment on attachment 676659 [details] [diff] [review]
Patch to fix

yes, belated approval - i had been looking for it to get on central first. thank you for getting it into beta in time.
Attachment #676659 - Flags: approval-mozilla-beta?
Attachment #676659 - Flags: approval-mozilla-beta+
Attachment #676659 - Flags: approval-mozilla-aurora?
Attachment #676659 - Flags: approval-mozilla-aurora+

Comment 9

5 years ago
Beautiful.  All works again in 17.0b4.  Do we need a console message that this method is going away in favour of the new one?  Or is it going to stay?
You need to log in before you can comment on or make changes to this bug.