Closed
Bug 807078
Opened 12 years ago
Closed 12 years ago
login and logout listeners not getting cleaned up after trusty ui close
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][feature-complete])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
The event listeners in the b2g/components/SignInToWebsite pipe object are not getting cleaned up properly because the pipe is not receiving the "finished" message from identity.js. This is causing the event callbacks to fire two, three, four, etc. times per message.
It appears that once gaia shuts down the identity.js window, no callbacks fire, so the close handlers don't get a chance to tell gecko to clean up.
Assignee | ||
Comment 1•12 years ago
|
||
Clean up multiple listeners by sending a finished message to gecko first, and then messaging gaia.
Attachment #676742 -
Flags: feedback?(zack.carter)
Updated•12 years ago
|
blocking-basecamp: --- → ?
Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 676742 [details] [diff] [review]
clean up message listeners before closing window
Review of attachment 676742 [details] [diff] [review]:
-----------------------------------------------------------------
This fixes a problem that emerged when we started testing against the trusty ui. There may have been a race condition all along, but it wasn't evident to me with the regular popup
Attachment #676742 -
Flags: feedback?(zack.carter) → review?(benadida)
Comment 4•12 years ago
|
||
Comment on attachment 676742 [details] [diff] [review]
clean up message listeners before closing window
Review of attachment 676742 [details] [diff] [review]:
-----------------------------------------------------------------
r+ with tiny comment nit below.
::: b2g/chrome/content/identity.js
@@ +62,2 @@
> /*
> * Notify the UI to close the dialog and return to the caller application
if you're removing the callback, change this comment.
Updated•12 years ago
|
Blocks: basecamp-id
Assignee | ||
Comment 5•12 years ago
|
||
changed comment per Comment #4 r=benadida
Attachment #676742 -
Attachment is obsolete: true
Attachment #676742 -
Flags: review?(benadida)
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 7•12 years ago
|
||
New patch fixes collision with m-c
Attachment #677157 -
Attachment is obsolete: true
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Ryan VanderMeulen from comment #6)
> This doesn't apply cleanly to m-c. Please rebase.
Thanks, Ryan. Should be good to go, now.
Keywords: checkin-needed
Assignee | ||
Comment 9•12 years ago
|
||
Updated•12 years ago
|
Whiteboard: [LOE:M] → [LOE:M][feature-complete]
Comment 10•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e7914709279
Should this have a test?
Flags: in-testsuite?
Keywords: checkin-needed
Comment 11•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment 12•12 years ago
|
||
I take it the right way to verify this is to go through the persona login flows and make sure I'm not seeing multiple unexpected events getting fired, right?
Keywords: verifyme
QA Contact: jsmith
Comment 13•12 years ago
|
||
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #12)
> I take it the right way to verify this is to go through the persona login
> flows and make sure I'm not seeing multiple unexpected events getting fired,
> right?
Yes. We're new to marionette tests; once we've got them figured out, there will be a test for this.
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•