Closed
Bug 1169706
Opened 10 years ago
Closed 10 years ago
Link-clicker UI broken for release builds (38 & earlier) - Join doesn't work (TypeError: rootNavigator.mediaDevices.enumerateDevices is not a function)
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox41 fixed)
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
Details
(Keywords: regression)
Attachments
(1 file)
3.56 KB,
patch
|
dmosedale
:
review+
|
Details | Diff | Splinter Review |
The loop-client link clicker UI is currently broken in FF 38 & earlier.
STR:
1) Get a Hello link
2) Open it in FF 38 or earlier
3) Click Join
Expected Results
=> Device permission prompt displayed
Actual Results
=> Nothing happens, error on console:
"[Dispatcher] Dispatching action caused an exception: " TypeError: rootNavigator.mediaDevices.enumerateDevices is not a function
Stack trace:
hasAudioOrVideoDevices@http://localhost:3000/content/shared/js/utils.js:318:7
ActiveRoomStore<.joinRoom@http://localhost:3000/content/shared/js/activeRoomStore.js:438:1
Dispatcher.prototype._dispatchNextAction/<@http://localhost:3000/content/shared/js/dispatcher.js:74:11
Dispatcher.prototype._dispatchNextAction@http://localhost:3000/content/shared/js/dispatcher.js:72:7
Dispatcher.prototype.dispatch@http://localhost:3000/content/shared/js/dispatcher.js:46:7
StandaloneRoomView<.joinRoom@http://localhost:3000/content/js/standaloneRoomViews.js:421:7
executeDispatch@http://localhost:3000/content/shared/libs/react-0.12.2.js:3282:21
[95]</SimpleEventPlugin.executeDispatch@http://localhost:3000/content/shared/libs/react-0.12.2.js:15905:23
forEachEventDispatch@http://localhost:3000/content/shared/libs/react-0.12.2.js:3270:5
executeDispatchesInOrder@http://localhost:3000/content/shared/libs/react-0.12.2.js:3291:3
[19]</executeDispatchesAndRelease@http://localhost:3000/content/shared/libs/react-0.12.2.js:2666:5
[126]</forEachAccumulated@http://localhost:3000/content/shared/libs/react-0.12.2.js:17883:5
[19]</EventPluginHub.processEventQueue@http://localhost:3000/content/shared/libs/react-0.12.2.js:2871:5
runEventQueueInBatch@http://localhost:3000/content/shared/libs/react-0.12.2.js:10391:3
[62]</ReactEventEmitterMixin.handleTopLevel@http://localhost:3000/content/shared/libs/react-0.12.2.js:10417:5
handleTopLevelImpl@http://localhost:3000/content/shared/libs/react-0.12.2.js:10503:1
[107]</Mixin.perform@http://localhost:3000/content/shared/libs/react-0.12.2.js:16876:13
[54]</ReactDefaultBatchingStrategy.batchedUpdates@http://localhost:3000/content/shared/libs/react-0.12.2.js:9142:7
batchedUpdates@http://localhost:3000/content/shared/libs/react-0.12.2.js:15103:3
[63]</ReactEventListener.dispatchEvent@http://localhost:3000/content/shared/libs/react-0.12.2.js:10598:7
Assignee | ||
Updated•10 years ago
|
Severity: normal → blocker
Assignee | ||
Comment 1•10 years ago
|
||
The issue is that navigator.mediaDevices doesn't have enumerateDevices in 38, nor does window.MediaStreamTrack have getSources.
Hence we fail :-(
This adds in checks and appropriate unit tests.
Attachment #8613221 -
Flags: review?(mdeboer)
Attachment #8613221 -
Flags: review?(dmose)
Comment 2•10 years ago
|
||
Comment on attachment 8613221 [details] [diff] [review]
Add checks to see if the functions exist in the objects or not.
Review of attachment 8613221 [details] [diff] [review]:
-----------------------------------------------------------------
If I understand correctly, in older versions, this reverts us to the older behavior of simply assuming that audio will work, and if it won't, that's the way it goes, because that's the best the old APIs will allow (without manually making a call to see if it fails)?
r=dmose (though I haven't hand tested).
::: browser/components/loop/content/shared/js/utils.js
@@ +311,5 @@
> *
> * @param {Function} callback Called with a boolean which is true if there
> * are audio devices present.
> */
> function hasAudioOrVideoDevices(callback) {
The name is impressively confusing for readers of call-site code. Can you file a bug on that (I'm assuming it makes sense to delay the change given the time-pressure associated with this deploy).
Attachment #8613221 -
Flags: review?(dmose) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8613221 -
Flags: review?(mdeboer)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #2)
> Comment on attachment 8613221 [details] [diff] [review]
> If I understand correctly, in older versions, this reverts us to the older
> behavior of simply assuming that audio will work, and if it won't, that's
> the way it goes, because that's the best the old APIs will allow (without
> manually making a call to see if it fails)?
Yes, we revert to the older behavior. We can't do anything more than that.
> ::: browser/components/loop/content/shared/js/utils.js
> > function hasAudioOrVideoDevices(callback) {
>
> The name is impressively confusing for readers of call-site code. Can you
> file a bug on that (I'm assuming it makes sense to delay the change given
> the time-pressure associated with this deploy).
I can't see anything confusing here. The only confusing bit is that the call site callback parameter is hasAudio, and should have changed to hasDevices. I can change that before landing it as its just a rename.
Comment 5•10 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #3)
> I can't see anything confusing here. The only confusing bit is that the call
> site callback parameter is hasAudio, and should have changed to hasDevices.
> I can change that before landing it as its just a rename.
Yeah, the problem is a combination of that, and the fact that the doxygen stuff about the function only talks about audio, not video.
Comment 6•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•