Disable/override the "Please allow camera & mic access" dialog from the SDK

VERIFIED FIXED in mozilla33

Status

Hello (Loop)
Client
P2
normal
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: standard8, Assigned: NiKo)

Tracking

unspecified
mozilla33
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 8432388 [details]
Dialog from the SDK

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

4 years ago
need UI for replacing this non-localized (see attachment) tokbox message.
Flags: needinfo?(dhenein)
Priority: -- → P2

Comment 2

4 years ago
Need to check with TokBox about how they want us to suppress/override/localize this dialog.
Flags: needinfo?(adam)

Comment 3

4 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: nobody → nperriault
Created attachment 8434256 [details] [diff] [review]
bug-1018875-suppress-ot-dialog.patch

Patch attached.
Attachment #8434256 - Flags: review?(standard8)
(Reporter)

Comment 5

4 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

4 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)
Created attachment 8434952 [details] [diff] [review]
bug-1018875-disable-ot-gum-dialog-2.patch

Cleaned up patch.
Attachment #8434256 - Attachment is obsolete: true
Attachment #8434952 - Flags: review?(standard8)
Created attachment 8434958 [details] [diff] [review]
bug-1018875-disable-ot-gum-dialog-3.patch

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

4 years ago
Attachment #8434958 - Flags: review?(standard8) → review+
(Reporter)

Comment 9

4 years ago
https://hg.mozilla.org/projects/elm/rev/ea180661e23b

Closing for tracking purposes
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: mozilla33 → mozilla32
(Reporter)

Comment 10

4 years ago
https://hg.mozilla.org/mozilla-central/rev/ea180661e23b
(Reporter)

Updated

4 years ago
Target Milestone: mozilla32 → mozilla33
Does this need manual testing or is testsuite coverage sufficient?
Flags: qe-verify?
Mark, can you please answer comment 11?
Flags: needinfo?(standard8)
(Reporter)

Comment 13

3 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)
Flags: qe-verify? → qe-verify+
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.