Closed Bug 1563994 Opened 5 months ago Closed 4 months ago

[10.15] Right Click When Not Focused Presents "Keystroke Recording" Access Grant Request Dialog

Categories

(Core :: Widget: Cocoa, defect, P1)

67 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 69+ verified
firefox-esr68 69+ verified
firefox68 + wontfix
firefox69 + verified
firefox70 + verified

People

(Reporter: steven, Assigned: haik)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

  1. Copied text from a page in Firefox
  2. Pasted text into conversation in Messages app.

Actual results:

Got prompted to give Firefox "Keystroke Recording" for all applications access.

Expected results:

Should have just copied the text, and nothing more.

I should note I'm on Catalina 10.15 and Firefox 67.0.4

I have seen this as well running 10.15, although I am not sure exactly how I triggered it.

Summary: Copy and Pasting from Firefox to Messages Presents "Keystroke Recording" Access Grant Request Dialog → [10.15] Copy and Pasting from Firefox to Messages Presents "Keystroke Recording" Access Grant Request Dialog

[Tracking Requested - why for this release]:
Firefox 69 comes out in September, 70 in October. Mojave was released late-ish September 2018, so if we're unlucky, 69 is what's out at the Catalina release, and this dialog is a PR nightmare waiting to happen (even if not an accurate reflection of what Firefox is doing). We should investigate it before general release, esp. in case this is an Apple bug of sorts.

Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Cocoa
Ever confirmed: true
Product: Firefox → Core

Markus or Stephen, any ideas what's triggering this?

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(mstange)

This seem to appear even when not doing copy and paste: https://twitter.com/jedisct1/status/1149293698708123648

Adding :haik.

I'll look into this. Firefox 67.0.2+ and 68 are not Notarized and are going to cause some unexpected behavior on 10.15 right now. We will be uplifting Notarization to 68 soon. I'll first see if this reproduces with Notarized builds.

Assignee: nobody → haftandilian

The only thing that comes to my mind is the event tap code around here which runs when a (non-native) context menu is open. It sets up a global listener for mouse down events so that we can close the menu when somebody clicks outside of any of our own windows.
I'm not aware of any global keyboard event handlers. A search for addGlobalMonitorForEventsMatchingMask returns no results.
Maybe this is related to IME things? Masayuki, any ideas?

Do we know what's on the Firefox main thread callstack when this dialog is shown? If the dialog is blocking us synchronously, it should be easy to find out the trigger call stack using a debugger, activity monitor, or even the Gecko profiler.

Flags: needinfo?(mstange) → needinfo?(masayuki)

I haven't been able to reproduce this so far.

(In reply to Markus Stange [:mstange] from comment #8)

The only thing that comes to my mind is the event tap code around here which runs when a (non-native) context menu is open. It sets up a global listener for mouse down events so that we can close the menu when somebody clicks outside of any of our own windows.

I think you're on to something, Markus. This is new in Catalina and announced at WWDC 2019 in the "Advances in macOS Security" presentation[1]. From the slides, it looks like it can be triggered by tapCreate and we do use CGEventTapCreate().

https://developer.apple.com/videos/play/wwdc2019/701/

Flags: needinfo?(mstange)
Assignee: haftandilian → nobody

Updating fx69/fx70 statuses based on comment #7. Haik, did you try reproducing with 67/68, or only with notarized 69/70 builds?

Flags: needinfo?(spohl.mozilla.bugs)

(In reply to :Gijs (he/him) from comment #11)

Updating fx69/fx70 statuses based on comment #7. Haik, did you try reproducing with 67/68, or only with notarized 69/70 builds?

Per comment 10, I don't think this is related to Notarization. I have tested on Firefox 68 (non-Notarized) and Firefox 69 (Notarized) without any luck getting the dialog. I've tried copying and pasting and right clicking in various places. (To run 68 on macOS 10.15, I downloaded it and cleared the quarantine attribute with $ xattr -r -d com.apple.quarantine /Applications/Firefox.app/.) For anyone trying to reproduce, you may have to run $ tccutil reset All first. Otherwise, the OS will remember your previous answer to the request and not prompt you again.

I'm not very knowledgeable about the API's mentioned in the WWDC presentation and by Markus on comment 8. I'll do some more research.

Although I don't know which API causes the prompt though, there are some doubtful points:

If user sets MOZ_LOG=TextInputHandler:3 (or bigger), here records all keyboard layouts/IMEs in the system:
https://searchfox.org/mozilla-central/rev/07f7390618692fa4f2a674a96b9b677df3a13450/widget/cocoa/TextInputHandler.mm#1669-1690
Of course, this shouldn't be enabled in environments of normal users.

And also we record all keyboard layout changes in all apps for ime-mode: active; and ime-mode: inactive;. This could cause this bug, but should be popped up the dialog at launching Firefox's main process:
https://searchfox.org/mozilla-central/rev/07f7390618692fa4f2a674a96b9b677df3a13450/widget/cocoa/TextInputHandler.mm#2982-2992

If the latter becomes a problem only when active keyboard layout is changed in other apps, I think that we can drop ime-mode: active; and ime-mode: inactive; supports since they are not supported by Linux due to API limitation, and they shouldn't be used in the web basically. Additionally, they are really unstable if user changes IMEs frequently (e.g., bilinguals of CJKT). E.g., IME state may be restored to unexpected language's IME.

Flags: needinfo?(masayuki)

Haik, we only install the event tap if the Firefox window is in the background. So try the following:

  1. Have a Firefox window open.
  2. Click on the Desktop so that the Firefox app loses focus (and Finder becomes the active app).
  3. Right click somewhere in the Firefox window.
Flags: needinfo?(mstange)

(In reply to Markus Stange [:mstange] from comment #14)

Haik, we only install the event tap if the Firefox window is in the background. So try the following:

[Edit, I can now reproduce this without selecting text.]

Thanks, that worked with the addition of selecting text. See below. The main thread doesn't appear to be involved in the display of the dialog. More debugging needed.

  1. $ tccutil reset All
  2. $ open /path/to/local/build/NightlyDebug.app
  3. Select some text in Firefox
  4. Click on the Desktop with Firefox still visible so that Firefox loses focus (and Finder becomes the active app).
  5. Right-click on the selected text in Firefox.
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00007fff70dc4396 libsystem_kernel.dylib`mach_msg_trap + 10
    frame #1: 0x00007fff70dc48fc libsystem_kernel.dylib`mach_msg + 60
    frame #2: 0x00007fff3979ed49 CoreFoundation`__CFRunLoopServiceMachPort + 322
    frame #3: 0x00007fff3979e2e5 CoreFoundation`__CFRunLoopRun + 1695
    frame #4: 0x00007fff3979d9c1 CoreFoundation`CFRunLoopRunSpecific + 499
    frame #5: 0x00007fff3836208d HIToolbox`RunCurrentEventLoopInMode + 292
    frame #6: 0x00007fff38361dcd HIToolbox`ReceiveNextEventCommon + 600
    frame #7: 0x00007fff38361b57 HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 64
    frame #8: 0x00007fff36a1c6b0 AppKit`_DPSNextEvent + 990
    frame #9: 0x00007fff36a1b424 AppKit`-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
    frame #10: 0x000000011674f5ac XUL`::-[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:](self=0x0000000109f4d660, _cmd="nextEventMatchingMask:untilDate:inMode:dequeue:", mask=18446744073709551615, expiration=0x00007fff9528a068, mode="kCFRunLoopDefaultMode", flag=YES) at nsAppShell.mm:169:24
    frame #11: 0x00007fff36a15b82 AppKit`-[NSApplication run] + 658
    frame #12: 0x0000000116751a17 XUL`nsAppShell::Run(this=0x000000010d660430) at nsAppShell.mm:703:5
    frame #13: 0x000000011948778b XUL`nsAppStartup::Run(this=0x000000010f432740) at nsAppStartup.cpp:276:30
    frame #14: 0x0000000119704546 XUL`XREMain::XRE_mainRun(this=0x00007ffee61dd3a8) at nsAppRunner.cpp:4636:22
    frame #15: 0x0000000119705696 XUL`XREMain::XRE_main(this=0x00007ffee61dd3a8, argc=3, argv=0x00007ffee61dda20, aConfig=0x00007ffee61dd548) at nsAppRunner.cpp:4771:8
    frame #16: 0x0000000119705ea9 XUL`XRE_main(argc=3, argv=0x00007ffee61dda20, aConfig=0x00007ffee61dd548) at nsAppRunner.cpp:4852:21
    frame #17: 0x000000011971ddd7 XUL`mozilla::BootstrapImpl::XRE_main(this=0x0000000109f121d0, argc=3, argv=0x00007ffee61dda20, aConfig=0x00007ffee61dd548) at Bootstrap.cpp:45:12
    frame #18: 0x0000000109a2357d firefox`do_main(argc=3, argv=0x00007ffee61dda20, envp=0x00007ffee61dda40) at nsBrowserApp.cpp:213:22
    frame #19: 0x0000000109a22f49 firefox`main(argc=3, argv=0x00007ffee61dda20, envp=0x00007ffee61dda40) at nsBrowserApp.cpp:295:16
    frame #20: 0x00007fff70c7bc35 libdyld.dylib`start + 1

The priority flag is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(spohl.mozilla.bugs)
Priority: -- → P2

Thanks! So that gives us reasonably high confidence that it is indeed the context menu event tap that is triggering this dialog.

I think there are few things to do here:

  • We should let Apple know that their dialog triggers even if the event mask does not contain key input events, which is unexpected.
  • We should check whether the more modern API, +[NSEvent addGlobalMonitorForEventsMatchingMask:handler:] has a similar problem or whether we can use it to listen for mouse down events and not get the dialog.
  • We can disable this particular piece of code on 10.15 and up. That means that in some cases, Firefox context menus might stay visible for longer than the user expects, basically until the user gives focus back to Firefox.

The "right" solution would be for us to stop using our own poor imitation of native context menus, and instead use true Cocoa context menus.

Doesn't sound like we're going to get a fix into an Fx68 dot release at this point. Let's aim for 69/68.1esr instead.

And to confirm this was caused by nsToolkit::RegisterForAllProcessMouseEvents(), I ran with MOZ_NO_OSX_EVENT_TAPS=1 and couldn't reproduce the problem.

$ MOZ_NO_OSX_EVENT_TAPS=1 open /Applications/Firefox\ Nightly.app

Summary: [10.15] Copy and Pasting from Firefox to Messages Presents "Keystroke Recording" Access Grant Request Dialog → [10.15] Right Click When Not Focused Presents "Keystroke Recording" Access Grant Request Dialog

(In reply to Markus Stange [:mstange] from comment #17)

I did a quick test with addGlobalMonitorForEventsMatchingMask listening for NSRightMouseDownMask | NSLeftMouseDownMask | NSOtherMouseDownMask and that did not trigger the dialog. I used a basic event handler that just printed something to the console for each click.

I'll take a crack at fixing this with addGlobalMonitorForEventsMatchingMask.

We reported the issue with CGEventTapCreate to Apple.

Assignee: nobody → haftandilian
Priority: P2 → P1

Review should be posted shortly. I've tested the change on 10.9, 10.14, and 10.15 Beta. With addGlobalMonitorForEventsMatchingMask, on macOS 10.15, the keystroke receiving dialog is no longer displayed.

Another difference is that, with the change, left-clicks outside the application also dismiss the context menu which did not happen before. I think this behavior is better and matches various native macOS applications I tested including Safari, Preview, Notes, and Google Chrome.

Use addGlobalMonitorForEventsMatchingMask instead of CGEventTapCreate to monitor for mouse clicks outside of the application while context menus are displayed.

Using addGlobalMonitorForEventsMatchingMask prevents the display of the privacy "Keystroke Receiving" dialog when listening for mouse clicks.

Pushed by haftandilian@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10fed3e6310a
[10.15] Right Click When Not Focused Presents "Keystroke Receiving" Access Grant Request Dialog r=mstange
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Please nominate this for Beta/ESR68/ESR60 approval when you get a chance.

Flags: needinfo?(haftandilian)

Comment on attachment 9081795 [details]
Bug 1563994 - [10.15] Right Click When Not Focused Presents "Keystroke Receiving" Access Grant Request Dialog r?mstange

Beta/Release Uplift Approval Request

  • User impact if declined: On macOS 10.15, if a user right-clicks on Firefox when it is in the background (not the focused application), macOS will display an alarming dialog implying that Firefox is recording keystrokes (when in fact Firefox is only listening for mouse clicks so it can roll up context menus.) This is likely to be cause for concern for some users.

The patch also fixes a minor macOS bug where if a context menu is opened by right clicking in Firefox when Firefox is in the background, the context menu is now dismissed when left clicking on another application.

  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: On macOS 10.15: Run "tccutil reset All". Then launch and position Firefox so that it is not the foreground application, but it is visible. Then right click in Firefox to display a context menu. On 10.15, without the fix, the "Keystroke Receiving" dialog will be displayed by macOS. With the fix, the dialog should not be displayed and a left click outside of Firefox should dismiss the context menu.

On other macOS versions, we should make sure the context menu works in the same way, but the "Keystroke Receiving" dialog only exists in 10.15.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change is fairly low risk because it is limited to macOS and is effectively replacing an older macOS API with a new macOS API.
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: The privacy/security dialog that is displayed on macOS 10.15 might be alarming or confusing to some users and give the impression Firefox is monitoring keystrokes for other applications which would be a security concern.
  • User impact if declined: On macOS 10.15, if a user right-clicks on Firefox when it is in the background (not the focused application), macOS will display an alarming dialog implying that Firefox is recording keystrokes (when in fact Firefox is only listening for mouse clicks so it can roll up context menus.) This is likely to be cause for concern for some users.

The patch also fixes a minor macOS bug where if a context menu is opened by right clicking in Firefox when Firefox is in the background, the context menu is now dismissed when left clicking on another application.

  • Fix Landed on Version: 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change is fairly low risk because it is limited to macOS and is effectively replacing an older macOS API with a new macOS API.
  • String or UUID changes made by this patch:
Flags: needinfo?(haftandilian)
Attachment #9081795 - Flags: approval-mozilla-release?
Attachment #9081795 - Flags: approval-mozilla-esr68?
Attachment #9081795 - Flags: approval-mozilla-esr60?
Attachment #9081795 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9081795 [details]
Bug 1563994 - [10.15] Right Click When Not Focused Presents "Keystroke Receiving" Access Grant Request Dialog r?mstange

Fixes a scary-looking warning dialog presented to users on macOS 10.15. Approved for 69.0b10.

Attachment #9081795 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Was there a particular reason that this patch was nominated for release approval too vs. the other 10.15 fixes which are shipping with 69?

Flags: needinfo?(haftandilian)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #28)

Was there a particular reason that this patch was nominated for release approval too vs. the other 10.15 fixes which are shipping with 69?

Sorry, release probably isn't required. It's not likely that 10.15 will ship before September 3rd when 69 releases. Since the 10.15 release date hasn't been announced yet, I expect it will be later.

Flags: needinfo?(haftandilian)

Comment on attachment 9081795 [details]
Bug 1563994 - [10.15] Right Click When Not Focused Presents "Keystroke Receiving" Access Grant Request Dialog r?mstange

Thanks for confirming. Let's stick to letting this ride with 69 for now then.

Attachment #9081795 - Flags: approval-mozilla-release? → approval-mozilla-release-
QA Whiteboard: [qa-triaged]

Reproduced the initial issue using old Nightly before the fix. Verified that the Keystroke dialog does not appear anymore on latest Nightly (70.0a1) and latest Beta 69.0b10 (from treeherder) using macOS 10.15 (19A512f).

Comment on attachment 9081795 [details]
Bug 1563994 - [10.15] Right Click When Not Focused Presents "Keystroke Receiving" Access Grant Request Dialog r?mstange

fix for macos 10.15, verified in beta, approved for 68.1 and 60.9

Attachment #9081795 - Flags: approval-mozilla-esr68?
Attachment #9081795 - Flags: approval-mozilla-esr68+
Attachment #9081795 - Flags: approval-mozilla-esr60?
Attachment #9081795 - Flags: approval-mozilla-esr60+

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #32)

Reproduced the initial issue using old Nightly before the fix. Verified that the Keystroke dialog does not appear anymore on latest Nightly (70.0a1) and latest Beta 69.0b10 (from treeherder) using macOS 10.15 (19A512f).

Also checked on CI builds for 60.9esr and 68.1esr and they are also fixed using the same version of macOS 10.15.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Blocks: 1594028
See Also: → 1594429
You need to log in before you can comment on or make changes to this bug.