1731898 - Skip the application chooser dialog step when a non-standard protocol has registered handler

Status: RESOLVED FIXED

(Firefox :: File Handling, enhancement, P2)

Description

[Sam Foster [:sfoster] (he/him)]

We want to adjust the protocol handling logic and flow to entirely skip the dialog that asks users to choose an application when there is a registered handler.

Comment 1

[Bernard Igiri]

Would this be a reversal of Bug 1565574?
https://bugzilla.mozilla.org/show_bug.cgi?id=1565574

Comment 2

[Sam Foster [:sfoster] (he/him)]

I'm going to redirect to Gijs. Looking at 1565574 it would guess we don't want to just reverse that work, but he'll know more.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Sorry, mis-click. See comment 1 @gijs

Comment 4

[:Gijs (he/him)]

(In reply to Bernard Igiri from comment #1)

Would this be a reversal of Bug 1565574?
https://bugzilla.mozilla.org/show_bug.cgi?id=1565574

No?

This bug is about removing the "what application would you like to use" dialog, that bug added the "Are you sure you want to allow example.com to open the foo: protocol/scheme" dialog. The latter needs to stay, the former needs to go, for this specific set of protocols where we have a per-protocol application. The latter's wording wants updating to indicate which application we'll open if the user grants permission to the site.

This means that for e.g. zoom (and ms-teams, and ...), users will get 1 dialog instead of 2.

Comment 5

[:Gijs (he/him)]

Please have a look at https://www.figma.com/file/cunx7mGzJVW8TdtiKaBhkq/MR1.1-%2F-MR2---Desktop?node-id=1%3A9 if you haven't already and do ask more questions for things that are unclear.

Comment 6

[Bernard Igiri]

Would callto count as a non-standard protocol? I tested that with <a href="callto:16509030800" title="trigger Skype">callto:16509030800</a> and Skype launched as expected once I selected both the "Always allow this domain" and "Always use this app" checkboxes. I attempted to use <a href="zoomus:94012610848" title="trigger Zoom">zoomus:94012610848</a> to test Zoom but that did nothing when I clicked it. Do you have an example URI that I should test?

Comment 7

[:Gijs (he/him)]

(In reply to Bernard Igiri from comment #6)

Would callto count as a non-standard protocol?

To start with we should consider the list in https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler as standard, and everything else as non-standard, I think. We can overlay onto that list if we need to, but that seems like a reasonable start. That list is already in code in at least 2 places, so ideally we can reuse one of those.

I tested that with <a href="callto:16509030800" title="trigger Skype">callto:16509030800</a> and Skype launched as expected once I selected both the "Always allow this domain" and "Always use this app" checkboxes.

Sure, but the point of this bug is that we want a better experience than 2 dialogs + 2 checkboxes - just 1 dialog with 1 checkbox, and not prompting the user for an application, only for the site permission (still with a checkbox).

I attempted to use <a href="zoomus:94012610848" title="trigger Zoom">zoomus:94012610848</a> to test Zoom but that did nothing when I clicked it. Do you have an example URI that I should test?

zoommtg is the protocol zoom uses, I think - but you should be able to use the (https) URL from any of your scheduled meeting calendar invites and then check what external protocol URI gets navigated to, e.g. by debugging the relevant dialog code.

Comment 8

[Bernard Igiri]

Here are some useful Searchfox links in the event this work is picked up again in the future.

handleURI method which is called when a URI is loaded

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/ContentDispatchChooser.jsm#253-331

dialog prompts

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/ContentDispatchChooser.jsm#361-458

permissionDialog.js prompt is triggered here

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/ContentDispatchChooser.jsm#380

appChooser.js prompt is triggered here

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/ContentDispatchChooser.jsm#427

permissionDialog dialog

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/content/permissionDialog.js
• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/content/permissionDialog.xhtml

appChooser dialog

• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/content/appChooser.js
• https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/content/appChooser.xhtml

Comment 9

[Mike Kaply [:mkaply]]

I'm going to take a look at this. Something I ran into immediately with zoommtg URLs (on windows) is that we don't appear to launch Zoom if it isn't open and if it is open, all we do is focus it. So we're not passing the URL from:

zoommtg:https://mozilla.zoom.us/j/92162736037

to Zoom.

The zoommtg protocol handler looks like this:

"C:\Users\mike\AppData\Roaming\Zoom\bin\Zoom.exe" "--url=%1"

I don't think we're using the zoommtg->shell->open->command to pass anything to zoom.

Does anyone off hand know the place where we do the action of launching the application that we get from the OS protocol information?

Comment 10

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #9)

I'm going to take a look at this. Something I ran into immediately with zoommtg URLs (on windows) is that we don't appear to launch Zoom if it isn't open and if it is open, all we do is focus it. So we're not passing the URL from:

zoommtg:https://mozilla.zoom.us/j/92162736037

to Zoom.

Aren't we? I'd check that somehow before assuming that's not what we do. How else is the app becoming focused?

The zoommtg protocol handler looks like this:

"C:\Users\mike\AppData\Roaming\Zoom\bin\Zoom.exe" "--url=%1"

I don't think we're using the zoommtg->shell->open->command to pass anything to zoom.

Does anyone off hand know the place where we do the action of launching the application that we get from the OS protocol information?

https://searchfox.org/mozilla-central/rev/9d66919569655a8fae9b431550241c058fa85b8a/uriloader/exthandler/win/nsMIMEInfoWin.cpp#273, I think.

It works fine on macOS (but the code there will be slightly different).

Comment 11

[:Gijs (he/him)]

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

Does anyone off hand know the place where we do the action of launching the application that we get from the OS protocol information?

https://searchfox.org/mozilla-central/rev/9d66919569655a8fae9b431550241c058fa85b8a/uriloader/exthandler/win/nsMIMEInfoWin.cpp#273, I think.

It works fine on macOS (but the code there will be slightly different).

Called from https://searchfox.org/mozilla-central/source/uriloader/exthandler/nsMIMEInfoImpl.cpp#351 , fwiw - of course, if you've tried to manually select the zoom app then yeah, we won't use the shell command (we hit the "use helper app" path further down in that method).

Comment 12

[Sarah Clements [:sclements]]

Attachment: Bug 1731898 - Change protocol handling logic for non-standard cases

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/661c496ff0bd
Change protocol handling logic for non-standard cases r=Gijs

Comment 14

[Iulian Moraru]

Backed out for causing bc failures on browser_first_prompt_not_blocked_without_user_interaction.js.

Push with failures

Failure log

Backout link

[task 2022-04-13T18:08:29.653Z] 18:08:29     INFO - TEST-START | uriloader/exthandler/tests/mochitest/browser_first_prompt_not_blocked_without_user_interaction.js
[task 2022-04-13T18:08:29.687Z] 18:08:29     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 105a3a400 == 8 [pid = 4418] [id = 14]
[task 2022-04-13T18:08:29.688Z] 18:08:29     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 18 (1013a0350) [pid = 4418] [serial = 38] [outer = 0]
[task 2022-04-13T18:08:29.688Z] 18:08:29     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 19 (105a3b800) [pid = 4418] [serial = 39] [outer = 1013a0350]
[task 2022-04-13T18:08:29.719Z] 18:08:29     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 20 (105d15800) [pid = 4418] [serial = 40] [outer = 1013a0350]
[task 2022-04-13T18:08:29.783Z] 18:08:29     INFO - GECKO(4416) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /var/folders/c2/gp9_mxbj7px3dsk7fl0bpv5r000014/T/tmp4o13i0ui.mozrunner/runtests_leaks_tab_pid4497.log
[task 2022-04-13T18:08:29.784Z] 18:08:29     INFO - GECKO(4416) | [4497, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp:339
[task 2022-04-13T18:08:30.023Z] 18:08:30     INFO - GECKO(4416) | [Parent 4416: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 130363c00 == 13 [pid = 4416] [id = 44]
[task 2022-04-13T18:08:30.024Z] 18:08:30     INFO - GECKO(4416) | [Parent 4416: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 57 (10d031f60) [pid = 4416] [serial = 90] [outer = 0]
<...>
[task 2022-04-13T18:08:43.454Z] 18:08:43     INFO - GECKO(4416) | [Child 4490: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 4 (113171400) [pid = 4490] [serial = 2] [outer = 0] [url = about:blank]
[task 2022-04-13T18:09:59.805Z] 18:09:59     INFO - TEST-INFO | started process screencapture
[task 2022-04-13T18:09:59.923Z] 18:09:59     INFO - TEST-INFO | screencapture: exit 0
[task 2022-04-13T18:09:59.924Z] 18:09:59     INFO - Buffered messages logged at 18:08:29
[task 2022-04-13T18:09:59.924Z] 18:09:59     INFO - Entering setup bound test_common_initialize
[task 2022-04-13T18:09:59.925Z] 18:09:59     INFO - Leaving setup bound test_common_initialize
[task 2022-04-13T18:09:59.925Z] 18:09:59     INFO - Entering test bound setupMailHandler
[task 2022-04-13T18:09:59.926Z] 18:09:59     INFO - Leaving test bound setupMailHandler
[task 2022-04-13T18:09:59.926Z] 18:09:59     INFO - Entering test bound test_open_without_user_interaction
[task 2022-04-13T18:09:59.926Z] 18:09:59     INFO - Buffered messages logged at 18:08:30
[task 2022-04-13T18:09:59.927Z] 18:09:59     INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.com/browser/uriloader/exthandler/tests/mochitest/file_external_protocol_iframe.html" line: 0}]
[task 2022-04-13T18:09:59.927Z] 18:09:59     INFO - Buffered messages finished
[task 2022-04-13T18:09:59.928Z] 18:09:59     INFO - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/mochitest/browser_first_prompt_not_blocked_without_user_interaction.js | Test timed out - 
[task 2022-04-13T18:09:59.929Z] 18:09:59     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-04-13T18:09:59.930Z] 18:09:59     INFO - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/mochitest/browser_first_prompt_not_blocked_without_user_interaction.js | Uncaught exception received from previously timed out test - dialogopen listener not removed before the end of test
[task 2022-04-13T18:09:59.930Z] 18:09:59     INFO - GECKO(4416) | MEMORY STAT | vsize 9023MB | residentFast 416MB | heapAllocated 128MB
[task 2022-04-13T18:09:59.931Z] 18:09:59     INFO - TEST-OK | uriloader/exthandler/tests/mochitest/browser_first_prompt_not_blocked_without_user_interaction.js | took 90161ms
[task 2022-04-13T18:09:59.931Z] 18:09:59     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-04-13T18:09:59.932Z] 18:09:59     INFO - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/mochitest/browser_first_prompt_not_blocked_without_user_interaction.js | Found a tab after previous test timed out: https://example.com/browser/uriloader/exthandler/tests/mochitest/file_external_protocol_iframe.html - 
[task 2022-04-13T18:09:59.932Z] 18:09:59     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 101333400 == 1 [pid = 4418] [id = 15]
[task 2022-04-13T18:09:59.933Z] 18:09:59     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (10139dd40) [pid = 4418] [serial = 41] [outer = 0]
[task 2022-04-13T18:09:59.933Z] 18:09:59     INFO - GECKO(4416) | [Child 4418: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (101512000) [pid = 4418] [serial = 42] [outer = 10139dd40]
[task 2022-04-13T18:09:59.933Z] 18:09:59     INFO - checking window state
[task 2022-04-13T18:09:59.934Z] 18:09:59     INFO - TEST-START | uriloader/exthandler/tests/mochitest/browser_ftp_protocol_handlers.js

Comment 15

[Sarah Clements [:sclements]]

I made a minor change and the test in question that failed passes locally and on try. Going to re-land this pr.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0922c87ea803
Change protocol handling logic for non-standard cases r=Gijs

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/0922c87ea803