Closed
Bug 804932
Opened 12 years ago
Closed 12 years ago
Pass arbitrary options from RP callers to nsDOM up to BrowserID internal api methods
Categories
(Core Graveyard :: Identity, enhancement)
Core Graveyard
Identity
Tracking
(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)
People
(Reporter: jedp, Assigned: jedp)
References
Details
(Whiteboard: [LOE:M][qa-])
Attachments
(2 files, 5 obsolete files)
46.52 KB,
patch
|
Details | Diff | Splinter Review | |
46.06 KB,
patch
|
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
When an RP caller invokes a browserid method (watch, request, or logout), we should ensure that arbitrary parameters are passed all the way up to the browserid internal api calls.
Assignee | ||
Comment 1•12 years ago
|
||
This patch is to be applied over the patch for bug 794680. Local xpcshell tests pass. Small adjustments to browserid internal api methods must be made before we can do a full test.
Assignee | ||
Comment 2•12 years ago
|
||
Fixes some bugs in object duplication and option passing up to internal_api. Introduces new bug on logout: the dreaded JavaScript error: , line 0: Permission denied to access object
Attachment #674564 -
Attachment is obsolete: true
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #674718 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
tbpl: https://tbpl.mozilla.org/?tree=Try&rev=c8cdfb03b442
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 680964 [details] [diff] [review] pass rp caller params from nsDOM through and up to browserid Review of attachment 680964 [details] [diff] [review]: ----------------------------------------------------------------- Ben, how does this look for passing arbitrary RP parameters up and back through the pipe?
Attachment #680964 -
Flags: review?(benadida)
Comment 6•12 years ago
|
||
I see this is linked to bug 790141 - is this required to get bug 790141 working? If so, this should probably be basecamp+.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #6) > I see this is linked to bug 790141 - is this required to get bug 790141 > working? If so, this should probably be basecamp+. Yes, as I'm splitting some of that work out into this bug, that makes sense. I don't have the superpowers to flip that switch, though.
blocking-basecamp: --- → ?
Comment 8•12 years ago
|
||
Comment on attachment 680964 [details] [diff] [review] pass rp caller params from nsDOM through and up to browserid Review of attachment 680964 [details] [diff] [review]: ----------------------------------------------------------------- I *think* that this patch includes unrelated code, specifically the _signature stuff. r-, but tell me if I got it wrong. ::: b2g/chrome/content/identity.js @@ +54,5 @@ > * method: one of 'login', 'logout', 'ready' > * assertion: optional assertion > */ > function identityCall(message) { > + // If the calling RP context had a signature, include it in the message. is this from a different patch? I don't see the purpose of this code for this particular feature.
Attachment #680964 -
Flags: review?(benadida) → review-
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Ben Adida [:benadida] from comment #8) > Comment on attachment 680964 [details] [diff] [review] > pass rp caller params from nsDOM through and up to browserid > > Review of attachment 680964 [details] [diff] [review]: > ----------------------------------------------------------------- > > I *think* that this patch includes unrelated code, specifically the > _signature stuff. r-, but tell me if I got it wrong. > > ::: b2g/chrome/content/identity.js > @@ +54,5 @@ > > * method: one of 'login', 'logout', 'ready' > > * assertion: optional assertion > > */ > > function identityCall(message) { > > + // If the calling RP context had a signature, include it in the message. > > is this from a different patch? I don't see the purpose of this code for > this particular feature. Quite right. This was sloppy of me. The signature stuff belongs in Bug 790141. I shall fix and re-submit.
Assignee | ||
Comment 10•12 years ago
|
||
As per Comment #8, removes '_signature' stuff that belongs in the patch for Bug 790141
Attachment #680964 -
Attachment is obsolete: true
Attachment #681311 -
Flags: review?(benadida)
Assignee | ||
Comment 11•12 years ago
|
||
Sorry, previous version missed two spots that needed scrubbing.
Attachment #681311 -
Attachment is obsolete: true
Attachment #681311 -
Flags: review?(benadida)
Attachment #681508 -
Flags: review?(benadida)
Comment 12•12 years ago
|
||
Comment on attachment 681508 [details] [diff] [review] pass rp caller params from nsDOM through and up to browserid Review of attachment 681508 [details] [diff] [review]: ----------------------------------------------------------------- great, thanks for cleaning this up!
Attachment #681508 -
Flags: review?(benadida) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 13•12 years ago
|
||
The hg changeset here doesn't have the right info - you need your author info there.
Keywords: checkin-needed
Comment 14•12 years ago
|
||
See https://bug781534.bugzilla.mozilla.org/attachment.cgi?id=678080 for an example of what needs to included in the changeset.
Assignee | ||
Comment 15•12 years ago
|
||
adding commit details to patch ...
Assignee | ||
Comment 16•12 years ago
|
||
added missing fields from patch header
Attachment #681508 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 17•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0b70eb5a351
Flags: in-testsuite+
Keywords: checkin-needed
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f0b70eb5a351
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•12 years ago
|
Whiteboard: [LOE:M] → [LOE:M][qa-]
Assignee | ||
Comment 19•12 years ago
|
||
Patch revised so it can be cleanly applied to beta.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 20•12 years ago
|
||
Even though this is closed, it should be marked bb+ for patch to be uplifted into beta
Comment 21•12 years ago
|
||
(In reply to Jed Parsons [:jparsons] from comment #20) > Even though this is closed, it should be marked bb+ for patch to be uplifted > into beta If it isn't basecamp+ btw, you can also nominate this for approval instead to get the uplift if it's +ed.
Assignee | ||
Comment 22•12 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #21) > (In reply to Jed Parsons [:jparsons] from comment #20) > > Even though this is closed, it should be marked bb+ for patch to be uplifted > > into beta > > If it isn't basecamp+ btw, you can also nominate this for approval instead > to get the uplift if it's +ed. I figured that since these are blockers of blockers, they are effectively bb+. I don't know what would be easier for everyone, but this seemed like the clearest route (even though it makes noise to stick a bb+ on a bunch of patches)
Assignee | ||
Updated•12 years ago
|
blocking-basecamp: ? → ---
Assignee | ||
Comment 23•12 years ago
|
||
Comment on attachment 689298 [details] [diff] [review] rereshed patch can be applied to beta [Approval Request Comment] Bug caused by (feature/regressing bug #): feature User impact if declined: native identity will fail in some cases Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low; isolated component String or UUID changes made by this patch: none
Attachment #689298 -
Flags: approval-mozilla-beta?
Updated•12 years ago
|
blocking-basecamp: --- → +
Updated•12 years ago
|
Attachment #689298 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 24•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/1a43b1b0ac96
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•