Closed Bug 1039263 Opened 7 years ago Closed 7 years ago

Loop standalone client can't restart a call (regression)


(Hello (Loop) :: Client, defect)

Not set


(Not tracked)



(Reporter: NiKo, Unassigned)


(Keywords: regression)


(1 file, 1 obsolete file)

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"


The new call won't start.


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]

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.
Attachment #8456875 - Attachment is obsolete: true
Attachment #8457834 - Flags: review?(standard8)
Assignee: nobody → nperriault
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla33
Closed: 7 years ago
Resolution: --- → FIXED
Attachment #8457834 - Flags: review?(standard8)
Given this was a regression, does this need a manual regression test or is the automation coverage sufficient?
Keywords: regression
Whiteboard: [qa?]
(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?
Flags: qe-verify+
QA Contact:
Whiteboard: [qa?]
You need to log in before you can comment on or make changes to this bug.