1511416 - Screen sharing preview pops up in Camera permission prompt (fun UI bug)!

Status: RESOLVED FIXED

(Firefox :: Site Permissions, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: ScreensharePreviewEscape.png

Once open, our in-prompt screen-sharing preview pops up in unexpected places!

STRs:
 1. Open https://jsfiddle.net/jib1/6zcpgb3p/
 2. Select a window from the list in the permission prompt, to see preview.
 3. Leave the prompt open, DON'T hit "Allow" or "Don't Allow".
 4. Open https://jsfiddle.net/jib1/r60bzmrs/ in a new tab.

Expected result:
- A boring prompt asking for camera and microphone.

Actual result:
- A prompt asking for camera and microphone with a screen sharing preview
  (see attached screenshot).

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

[Tracking Requested - why for this release]: Edge case that looks bad; not a regression.

Additionally, in Firefox 60 when I do this and switch back and forth between the tabs, both permission prompts disappear, except for the tiny triangle part. Resizing the window smaller brings them back.

Also, a review of how much code is shared when there are two prompts open simultaneously, might be wise from a security perspective.

Comment 2

[Johann Hofmann [:johannh]]

I don't think this is super critical to solve, so I'm not sure I agree that we need to track it.

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #1)
> Also, a review of how much code is shared when there are two prompts open
> simultaneously, might be wise from a security perspective.

I don't really understand the security aspect of it. It's a UI bug, but how could this kind of thing compromise user security? I guess we can check for other things like this anyway...

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Not a new issue, agreed that this doesn't need to track.

Comment 4

[Florian Quèze [:florian]]

Fun bug, probably easy to fix.

Comment 5

[Johann Hofmann [:johannh]]

To fix this, we need to look at this code: https://searchfox.org/mozilla-central/rev/00f3836a87b844b5e4bc82f698c559b9966e4be2/browser/modules/webrtcUI.jsm#492

This callback is called when e.g. switching between tabs and showing different permission panels. This code already does some cleanup here, but only when the doorhanger with the screensharing preview is removed.

What we want to do in that callback is hide webRTC-preview when the requestTypes does not include "Screen", so that it's hidden when switching tabs.

A test would be awesome.

Comment 6

[Johann Hofmann [:johannh]]

Also, that jsfiddle didn't work anymore for me, you can just open https://mozilla.github.io/webrtc-landing/gum_test.html in two tabs instead.

Comment 7

[akshitha shetty]

Attachment: Bug 1511416 -Screen sharing preview pops up in Camera permission prompt . r=johannh

Comment 8

[akshitha shetty]

Attachment: Bug 1511416 - Screen sharing preview pops up in camera permission prompt.

Comment 9

[akshitha shetty]

Attachment: Bug 1511416 -Screen sharing preview pops up in Camera permission prompt .

Comment 10

[akshitha shetty]

Hi, could you please guide me further by reviewing https://phabricator.services.mozilla.com/D21976.
I have tested the results and they seem to work well. Would be great to know if I am on right direction. Thanks.

Comment 11

[akshitha shetty]

I have modified it in the suggested direction. Thus, wanted to confirm.

Comment 12

[Johann Hofmann [:johannh]]

Reviewed, sorry for the delay, thanks :)

Comment 13

[akshitha shetty]

Thank you for the direction. Will update the revision on the suggested lines.

Comment 14

[Johann Hofmann [:johannh]]

Akshita, before landing this, it would be great if you could do a Try Push. Try is basically Continuous Integration for Firefox, making sure that tests are passing for your patch.

Getting access is outlined here: https://wiki.mozilla.org/ReleaseEngineering/TryServer#Getting_access_to_the_Try_Server

You will need to file a bug and need someone to vouch for you. I'd be happy to do that if you CC me on that bug.

After you've gotten access, to schedule a try run, you can use the handy ./mach try command. A possible syntax for this bug could be:

./mach try -b do -p win64,linux64,macosx64 -u mochitests -t none

(runs all mochitests on Windows, OSX and Linux)

Let me know if you need any help with that!

Comment 15

[akshitha shetty]

Sure! Thank you,
I have filled a bug here https://bugzilla.mozilla.org/show_bug.cgi?id=1536277. Will take it ahead once granted access.

Comment 16

[akshitha shetty]

Hi, so on carrying out the test the results are such - https://treeherder.mozilla.org/testview.html?repo=try&revision=165c2962444ca3bdb3e5f08b8c9533775c6e9e7e. Could you please let me know how go ahead with this ?

Comment 17

[Johann Hofmann [:johannh]]

That seems good, you can set checkin-needed now!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/891e21499044
Screen sharing preview pops up in Camera permission prompt . r=johannh

Comment 19

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/891e21499044