[Regression] Clicking on audioVideoButton / sharing-icon don't work when typed on urlbar

VERIFIED FIXED in Firefox 53

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: 684sigma, Assigned: nhnt11)

Tracking

({regression, ux-control})

51 Branch
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 wontfix, firefox-esr52 wontfix, firefox53+ verified, firefox54 verified, firefox55 verified)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 attachment)

I have a problem with Firefox Beta 52. It doesn't happen on Firefox ESR 45.
Sometimes when I click on permissions block floating at the top of the screen to open exact tab that has permission to use my microphone/camera, sometimes it opens popup with site permissions, and sometimes it doesn't.
It happens unpredictably, however, I noticed one specific scenario when it happens

1. Open https://mozdevs.github.io/MediaRecorder-examples/record-live-audio.html , allow to use the microphone
2. Select all text in location bar, type "."
3. Click on the right side of floating block with camera/microphone permissions

Result: Firefox window blinks (looses focus, then gets focus)
Expected: Panel with site permissions should open

Floating block with camera/microphone permissions is a small rectangle that appears at the top of the screen and contains of 2 halves: the left side displays Firefox logo, the right part displays white microphone/camera icon on orange background
Blocks: 1206233
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Device Permissions
Ever confirmed: true
Summary: Regression Clicking on floating block with camera/microphone permissions doesn't open popup with permissions sometimes → [Regression] Clicking on audioVideoButton / sharing-icon don't work when typed on urlbar
Version: 52 Branch → 51 Branch
Thanks for reporting this, I agree that it should be possible to cancel the camera/microphone access even if the urlbar is in "invalid" state. We encountered this problem in bug 1300755 but decided that showing the security indicators while the urlbar is invalid is too much of a security risk since it enables much easier URL spoofing.

A solution for this that came up in our discussion of this bug was to reset the urlbar state and open the identity popup when the user clicks on the (i) icon while a sharing indicator is shown and pageproxystate=invalid. The potential data loss is not ideal but I think it's the best compromise in this situation and this is still pretty much an edge case.
Whiteboard: [fxprivacy] [triage]
Component: Device Permissions → Site Identity and Permission Panels
(In reply to Johann Hofmann [:johannh] from comment #1)
> Thanks for reporting this, I agree that it should be possible to cancel the
> camera/microphone access even if the urlbar is in "invalid" state. We
> encountered this problem in bug 1300755 but decided that showing the
> security indicators while the urlbar is invalid is too much of a security
> risk since it enables much easier URL spoofing.
> 
> A solution for this that came up in our discussion of this bug was to reset
> the urlbar state and open the identity popup when the user clicks on the (i)
> icon while a sharing indicator is shown and pageproxystate=invalid. The
> potential data loss is not ideal but I think it's the best compromise in
> this situation and this is still pretty much an edge case.

Possible to reset (when opened) and revert (when closed)?
Iteration: --- → 55.1 - Mar 20
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Philipp, your input needed on this use case.  Take a look at the proposal in Comment 1.
Flags: needinfo?(philipp)
I think reverting makes sense here.
We should be able to handle this better once we have permissions UI exposed in preferences, to which we can then link instead of the doorhanger. But reverting the URL sounds like a decent interim solution.
Flags: needinfo?(philipp)
Track 53+ as regression.
Hi :johannh,
According to comment #4, can you help find an owner for this?
Flags: needinfo?(jhofmann)
Yes, we're taking care of it. Thanks :)
Flags: needinfo?(jhofmann)
Whiteboard: [fxprivacy] → [fxprivacy] [triage]
Assignee: nobody → nhnt11
Iteration: 55.1 - Mar 20 → 55.2 - Apr 3
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Comment on attachment 8852578 [details]
Bug 1345687 - Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site.

https://reviewboard.mozilla.org/r/124766/#review127602

Thanks!

::: browser/base/content/browser.js:7444
(Diff revision 3)
> -    // Don't allow left click, space or enter if the location has been modified.
> -    if (gURLBar.getAttribute("pageproxystate") != "valid") {
> +    // Don't allow left click, space or enter if the location has been modified,
> +    // so long as we're not sharing any devices.
> +    // If we are sharing a device, the identity block cannot be focused (and
> +    // therefore, interacted with) by the user. However, we want to allow opening
> +    // the identity popup from the device control menu, which calls click() on the
> +    // identity button, so we don't return early.

You should mention that the identity block should be disabled via CSS so that this doesn't affect the case of the user directly clicking into it.

::: browser/base/content/browser.js:7446
(Diff revision 3)
> +    // If we are sharing a device, the identity block cannot be focused (and
> +    // therefore, interacted with) by the user. However, we want to allow opening
> +    // the identity popup from the device control menu, which calls click() on the
> +    // identity button, so we don't return early.
> +    if (!this._sharingState &&
> +        gURLBar.getAttribute("pageproxystate") != "valid") {

I don't really like that we're doing this to work around the fact that other code just calls identityBlock.click()

Ideally we wouldn't have to do this and just use an API call like showPopup to show the popup.

But I guess this is broken in beta and this solution is likely the most conflict-free way to get it uplifted.
Attachment #8852578 - Flags: review?(jhofmann) → review+
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/665c55716bf6
Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site. r=johannh
https://hg.mozilla.org/mozilla-central/rev/665c55716bf6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Is this something we should consider backporting to affected branches or can it ride the trains?
Flags: needinfo?(nhnt11)
Comment on attachment 8852578 [details]
Bug 1345687 - Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site.

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: If the URL bar text value has been modified, no way to open the identity popup and revoke device permissions.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: yes. STR in comment 0. On macOS, you can navigate to the site linked in comment 0, allow the microphone, modify the URL bar text, and then in the microphone menu  (appears somewhere at the top right of the macOS menubar) choose "Control Sharing". Without the patch, nothing happens. With the patch, the identity popup is shown and from there the permission can be revoked.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simply allows programmatic click()'s to be called when sharing a device even if pageproxystate is invalid.
[String changes made/needed]: none
Attachment #8852578 - Flags: approval-mozilla-beta?
Attachment #8852578 - Flags: approval-mozilla-aurora?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)
> Is this something we should consider backporting to affected branches or can
> it ride the trains?

I had the approval request all filled out but apparently left the tab open without submitting it. smh.
Flags: needinfo?(nhnt11)
Build ID: 20170402030202
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

Verified as fixed on Firefox Nightly 55.0a1 on Windows 10 x 64, Windows 7 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Comment on attachment 8852578 [details]
Bug 1345687 - Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site.

Fix a regression and was verified. Aurora54+ & Beta53+.
Attachment #8852578 - Flags: approval-mozilla-beta?
Attachment #8852578 - Flags: approval-mozilla-beta+
Attachment #8852578 - Flags: approval-mozilla-aurora?
Attachment #8852578 - Flags: approval-mozilla-aurora+
Build ID: 20170404004003
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

Verified as fixed on Firefox Aurora 54.0a2 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
I will verify this also on Beta when the build with the fix will be available.
Build ID: 20170404030727
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0 

Verified as fixed on Firefox Beta tinderbox-builds 53.0b9 on 
Windows 10 x 64 (https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-win64/1491300447/) and on Mac OS X 10.12 (https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-macosx64/1491300447/).
I will test on Ubuntu 16.04 x64 when the build with the fix will be available.
Build ID: 20170407033734
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

Verified as fixed on Firefox Beta 53.0b10 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Comment on attachment 8852578 [details]
Bug 1345687 - Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site.

See comment 16.
Attachment #8852578 - Flags: approval-mozilla-esr52?
Comment on attachment 8852578 [details]
Bug 1345687 - Allow identity popup to be shown irrespective of pageproxystate if a permission has been granted to the current site.

I don't think this is a severe enough problem to warrant porting a fix to ESR52. We typically limit ESR uplifts to critical regressions and security issues. This is what ESR users warrant, dot releases that are stable and consistent quality. 

If a user runs into this regression (which seems like an edge case, see comment 1), they will workaround it by opening up the same site in a new tab and changing device sharing permission on the doorhanger UI without updating URL bar in parallel.
Attachment #8852578 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52-
You need to log in before you can comment on or make changes to this bug.