Closed
Bug 1018875
Opened 10 years ago
Closed 10 years ago
Disable/override the "Please allow camera & mic access" dialog from the SDK
Categories
(Hello (Loop) :: Client, defect, P2)
Hello (Loop)
Client
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla33
People
(Reporter: standard8, Unassigned)
References
Details
Attachments
(2 files, 2 obsolete files)
26.97 KB,
image/png
|
Details | |
5.77 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
On the standalone page, if you take too long to provide access to the camera and microphone you get a prompt displayed telling you want to do.
This is provided by the TokBox SDK, however, this doesn't support localisation and we may want better messages provided based on the browser etc.
Adding an image of the current dialog.
Comment 1•10 years ago
|
||
need UI for replacing this non-localized (see attachment) tokbox message.
Flags: needinfo?(dhenein)
Priority: -- → P2
Comment 2•10 years ago
|
||
Need to check with TokBox about how they want us to suppress/override/localize this dialog.
Flags: needinfo?(adam)
Comment 3•10 years ago
|
||
From TokBox:
You can suppress the dialog, information here: http://tokbox.com/opentok/libraries/client/js/release-notes.html#newFeatures
"The OpenTok.js library now presents user interface hints on how to grant access to the camera and microphone. You can prevent this user interface from being displayed by calling thepreventDefault() method of the event object in the accessDialogOpened and the accessDenied event handlers."
Flags: needinfo?(adam)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → nperriault
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8434256 [details] [diff] [review]
bug-1018875-suppress-ot-dialog.patch
Review of attachment 8434256 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, this looks good. I think the tests are reasonable, considering the limitations we have.
My only thought is if we want to explicitly check for event.preventDefault being called, but given your comment on the code, I don't think that's necessary.
::: browser/components/loop/content/shared/js/views.js
@@ +177,5 @@
> * @param {SessionConnectEvent} event
> */
> publish: function(event) {
> var outgoing = this.$(".outgoing").get(0);
> +
nit: several lines of whitespace in this function.
::: browser/components/loop/test/shared/views_test.js
@@ +119,5 @@
> sinon.assert.calledOnce(fakeSession.publish);
> });
> +
> + it("should start listening to OT publisher accessDialogOpened and " +
> + " accessDenied events",
nit: whitespace at EOL
@@ +146,5 @@
>
> sinon.assert.calledOnce(fakeSession.unpublish);
> });
> +
> + it("should unsubscribe from accessDialogOpened and accessDenied events",
nit: whitespace at EOL
Attachment #8434256 -
Flags: review?(standard8) → review+
Reporter | ||
Comment 6•10 years ago
|
||
(In reply to sescalante from comment #1)
> need UI for replacing this non-localized (see attachment) tokbox message.
Moved this request out to bug 1020253 which will implement the replacement UX once we've decided what it is.
Flags: needinfo?(dhenein)
Assignee | ||
Comment 7•10 years ago
|
||
Cleaned up patch.
Attachment #8434256 -
Attachment is obsolete: true
Attachment #8434952 -
Flags: review?(standard8)
Assignee | ||
Comment 8•10 years ago
|
||
This time with the right export. Sorry again.
Attachment #8434952 -
Attachment is obsolete: true
Attachment #8434952 -
Flags: review?(standard8)
Attachment #8434958 -
Flags: review?(standard8)
Reporter | ||
Updated•10 years ago
|
Attachment #8434958 -
Flags: review?(standard8) → review+
Reporter | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/projects/elm/rev/ea180661e23b
Closing for tracking purposes
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla33 → mozilla32
Reporter | ||
Comment 10•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Target Milestone: mozilla32 → mozilla33
Comment 11•10 years ago
|
||
Does this need manual testing or is testsuite coverage sufficient?
Flags: qe-verify?
Reporter | ||
Comment 13•10 years ago
|
||
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #11)
> Does this need manual testing or is testsuite coverage sufficient?
This can probably only be tested manually.
If when you start a call from the link clicker, you wait for about 5-10 seconds (that's probably all you get with the timeout), then you currently shouldn't see any notification of needing to accept the prompt.
There is somewhere a bug around for doing our own custom prompt, but we can address that when we fix that bug.
Flags: needinfo?(standard8)
Comment 14•10 years ago
|
||
I'm sorry this took so long, it fell off my radar. I've now tested this and it's working as expected.
Status: RESOLVED → VERIFIED
QA Contact: anthony.s.hughes
You need to log in
before you can comment on or make changes to this bug.
Description
•