Closed Bug 1039263 Opened 5 years ago Closed 5 years ago
Loop standalone client can't restart a call (regression)
Steps to reproduce: - Standalone webapp opens a loop call link - Clicks "Start the call" - Accepts media permissions - Hang up - Clicks again on "Start the call" Results: The new call won't start. Expected: The new call should start. Technical details: The console shows an error which is probably related: > Error: Invariant Violation: getDOMNode(): A component must be mounted to have a DOM node. localhost:3000/content/shared/libs/react-0.10.0.js:17415
Extracted conversation from IRC: [14:20:25] Standard8: NiKo`: it'd be interesting to know what regressed it [14:20:38] NiKo`: Standard8: porting it to react [14:20:43] Standard8: ah [14:21:02] NiKo`: Standard8: we were starting subscribing to model event when the component was about to mount [14:21:14] Standard8: weird though, I thought I tested all that [14:21:16] NiKo`: but the callback assumed that it was mounted already [14:21:20] NiKo`: race conditions as usual [14:21:37] NiKo`: so the fix is to start listening when the component is mounted [14:21:45] NiKo`: so we're sure to have a DOM [14:21:54] NiKo`: available for the component [14:21:56] Standard8: ah ok [14:23:02] NiKo`: actually we were assuming that componentWillMount was the equivalent of Backbone.View's initialize() [14:23:08] NiKo`: but it's componentDidMount really [14:23:42] NiKo`: (actually these are two different beasts, but componentDidMount is the closest to what initialize is) Also, switched to using Backbone.Events#listenTo and Backbone.Events#stopListening for consistency and ease of unregistration.
Attachment #8456875 - Flags: review?(dmose)
Comment on attachment 8456875 [details] [diff] [review] 0001-Bug-1039263-Fixed-standalone-Loop-UI-couldn-t-restar.patch Review of attachment 8456875 [details] [diff] [review]: ----------------------------------------------------------------- Looks good; thanks for the patch. r=dmose
Attachment #8456875 - Flags: review?(dmose) → review+
Addressed comments from Mark: [09:46:06] Standard8: NiKo`: there's an issue [09:46:27] Standard8: this.TypeError: event is undefined views.js:289 [09:46:32] Standard8: s/this.// [09:46:54] Standard8: NiKo`: looks like the streamCreated listener Added missing tests for this to prevent further unnoticed regressions.
Assignee: nobody → nperriault
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla33
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Given this was a regression, does this need a manual regression test or is the automation coverage sufficient?
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #6) > Given this was a regression, does this need a manual regression test or is > the automation coverage sufficient? This would be covered by a manual smoketest by making a call, or by the functional tests once they are automated (we have some functional tests in the tree now, but we've still got to automate them).
I confirm that this is working as expected in Firefox 33.0a2 and 34.0a1. Can someone please review the following smoketest to confirm it's sufficient? https://moztrap.mozilla.org/manage/case/14532/
Status: RESOLVED → VERIFIED
QA Contact: anthony.s.hughes
You need to log in before you can comment on or make changes to this bug.