1297336 - Device permission gone from the permission dropdown after detaching the tab to a new window

Status: VERIFIED FIXED

(Firefox :: Site Permissions, defect, P1)

Description

[Paul Silaghi, QA [:pauly]]

[Affected versions]:
- 51.0a1 (2016-08-22)

[Affected platforms]:
- all

[Steps to reproduce]:
1. Open 2 empty tabs
2. In one tab open http://permission.site/
3. Click on the Camera button and share the camera
4. Detach the camera tab to a new window
5. Open the control center

[Expected result]:
- camera permission displayed in the control center

[Actual result]:
- camera permission gone from the control center

[Regression range]:
- caused by bug 1206233

Comment 1

[Paul Silaghi, QA [:pauly]]

Attachment: no camera permission.png

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

NI Florian (author) and Johann (reviewer) on whether patches on bug 1206233 are something we should back out because of this regression, or if there would be a fix coming.

Comment 3

[Johann Hofmann [:johannh]]

We still have some time to fix this. Florian is on PTO this week but I'm sure we'll figure it out once he's back.

We forgot to put this one in triage though, thanks for the ping!

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch

Comment 5

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9cc4d63c189

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8788481 [details] [diff] [review]
Patch

Review of attachment 8788481 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the assertion fixed or removed. Thanks!

::: browser/base/content/test/webrtc/browser_devices_get_user_media_tear_off_tab.js
@@ +41,5 @@
> +    webrtcUI.showSharingDoorhanger(activeStreams[0], "Devices");
> +    ok(!win.gIdentityHandler._identityPopup.hidden,
> +       "control center should be open in the second window");
> +    ok(!win.gIdentityHandler._identityPopup.hidden,
> +       "control center should be hidden in the first window");

This is not asserting the thing it says. In fact I'm not sure this assertion needs to be made by your test at all, it doesn't seem related to the regression you're testing.

Comment 7

[Florian Quèze [:florian]]

(In reply to Johann Hofmann [:johannh] from comment #6)

> browser/base/content/test/webrtc/browser_devices_get_user_media_tear_off_tab.
> js
> @@ +41,5 @@
> > +    webrtcUI.showSharingDoorhanger(activeStreams[0], "Devices");
> > +    ok(!win.gIdentityHandler._identityPopup.hidden,
> > +       "control center should be open in the second window");
> > +    ok(!win.gIdentityHandler._identityPopup.hidden,
> > +       "control center should be hidden in the first window");
> 
> This is not asserting the thing it says. In fact I'm not sure this assertion
> needs to be made by your test at all, it doesn't seem related to the
> regression you're testing.

I fixed 2 regressions at once. The "webrtcUI.swapBrowserForNotification(otherBrowser, ourBrowser);" line in the patch fixes this problem: after tearing off a tab, when clicking "Control sharing" on the global indicator, it focused the first window and opened the control center panel there, instead of opening the control center panel in the new window. So yes, this test is related to the changes I'm making.

Comment 8

[Johann Hofmann [:johannh]]

(In reply to Florian Quèze [:florian] [:flo] from comment #7)
> 
> I fixed 2 regressions at once. The
> "webrtcUI.swapBrowserForNotification(otherBrowser, ourBrowser);" line in the
> patch fixes this problem: after tearing off a tab, when clicking "Control
> sharing" on the global indicator, it focused the first window and opened the
> control center panel there, instead of opening the control center panel in
> the new window. So yes, this test is related to the changes I'm making.

Oh I didn't realize it did that. Please fix the assert then :)

Comment 9

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/47080eb5264154cdc8726628981c1a7948eec96d
Bug 1297336 - Preserve device sharing indicator when tearing off a tab, r=johannh.

Comment 10

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/47080eb52641

Comment 11

[Adrian Florinescu [:adrian_sv]]

verified as fixed on 51.0a1 Build ID 20160913030425