Closed
Bug 806587
Opened 12 years ago
Closed 12 years ago
Rename getNoAppCodebasePrincipal back to getCodebasePrincipal
Categories
(Core :: Security: CAPS, defect)
Core
Security: CAPS
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: akeybl, Assigned: sicking)
Details
(Keywords: addon-compat)
Attachments
(1 file)
4.71 KB,
patch
|
mounir
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(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•12 years ago
|
||
Can we get this into tomorrow's beta to prevent any more add-on compat impact?
Updated•12 years ago
|
Keywords: addon-compat
Assignee | ||
Comment 2•12 years ago
|
||
[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 3•12 years ago
|
||
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+
Assignee | ||
Comment 4•12 years ago
|
||
> @@ +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.
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/434c6bdec0bb
Comment 6•12 years ago
|
||
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?
Updated•12 years ago
|
Flags: in-testsuite? → in-testsuite+
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/434c6bdec0bb
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment 8•12 years ago
|
||
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•12 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.
Description
•