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

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Mark Banner (:standard8)]

Attachment: 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

[:shell escalante]

need UI for replacing this non-localized (see attachment) tokbox message.

Comment 2

[Adam Roach [:abr]]

Need to check with TokBox about how they want us to suppress/override/localize this dialog.

Comment 3

[Adam Roach [:abr]]

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."

Comment 4

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: bug-1018875-suppress-ot-dialog.patch

Patch attached.

Comment 5

[Mark Banner (:standard8)]

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

Comment 6

[Mark Banner (:standard8)]

(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.

Comment 7

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: bug-1018875-disable-ot-gum-dialog-2.patch

Cleaned up patch.

Comment 8

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: bug-1018875-disable-ot-gum-dialog-3.patch

This time with the right export. Sorry again.

Comment 9

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/ea180661e23b

Closing for tracking purposes

Comment 10

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/ea180661e23b

Comment 11

[u279076]

Does this need manual testing or is testsuite coverage sufficient?

Comment 12

[u279076]

Mark, can you please answer comment 11?

Comment 13

[Mark Banner (:standard8)]

(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.

Comment 14

[u279076]

I'm sorry this took so long, it fell off my radar. I've now tested this and it's working as expected.