Closed Bug 1297336 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Site Permissions, defect, P1)

51 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 51
Iteration:
51.3 - Sep 19
Tracking Status
firefox48 --- unaffected
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- verified

People

(Reporter: pauly, Assigned: florian)

References

Details

(Keywords: regression, Whiteboard: [fxprivacy] )

Attachments

(2 files)

[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
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.
Flags: needinfo?(jhofmann)
Flags: needinfo?(florian)
Version: Trunk → 51 Branch
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!
Flags: needinfo?(jhofmann)
Flags: needinfo?(florian)
Whiteboard: [fxprivacy] [triage]
Priority: -- → P1
Assignee: nobody → florian
Status: NEW → ASSIGNED
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Attached patch PatchSplinter Review
Attachment #8788481 - Flags: review?(jhofmann)
Iteration: --- → 51.3 - Sep 12
Flags: qe-verify?
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.
Attachment #8788481 - Flags: review?(jhofmann) → review+
Flags: qe-verify? → qe-verify+
(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.
(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 :)
https://hg.mozilla.org/integration/fx-team/rev/47080eb5264154cdc8726628981c1a7948eec96d
Bug 1297336 - Preserve device sharing indicator when tearing off a tab, r=johannh.
https://hg.mozilla.org/mozilla-central/rev/47080eb52641
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
QA Contact: paul.silaghi
verified as fixed on 51.0a1 Build ID 20160913030425
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.