Do not allow more than one simultaneous "open external protocol" dialog to appear per originating tab
Categories
(Firefox :: File Handling, defect, P3)
Tracking
()
People
(Reporter: csasca, Assigned: Gijs)
References
(Depends on 1 open bug)
Details
Attachments
(5 files, 1 obsolete file)
Affected versions
- Firefox 68.4.0esr
- Firefox 70.0.1
- Firefox 72.0b11
- Firefox 73.0a1
Affected platforms
- Windows 10 & 7
- macOS 10.15
Steps to reproduce
- Open the following link and login with valid credentials
- If not available, create a new contact and add a phone number to it
- On the newly created contact, click on the phone number
Expected result
- An external application window requesting the user to choose is shown
Actual result
- Two windows will appear, requesting the same action
Regression range
- I will see for a regression, if there is one.
Additional notes
- The actual issue can be seen in the attachment.
- On Ubuntu 18, a request to launch an external application will not appear, instead a page stating that "The address was not understood" will appear.
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
This is an issue with Outlook, I'm pretty sure? It'll be navigating to tel:<phonenumber>
more than once.
I think we already have a bug on file about being able to open more than one of these dialogs, but I can't find it. Johann, do you remember? I would have thought we would have filed this after the zoommtg: stuff last year.
Comment 2•4 years ago
|
||
The only thing I know of is bug 1361653, which I originally thought was only an issue on Android. But if a website is able to open an infinite amount of these that sounds annoying on desktop, too. Not sure how to consolidate these bugs now.
Comment 3•4 years ago
|
||
This issue was reproducible on Windows 10 with Firefox Nightly version 74.0a1 (2020-01-14) (64-bit). Marked as affected.
Comment 4•4 years ago
|
||
The priority flag is not set for this bug.
:Gijs, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Updated•4 years ago
|
Comment 5•4 years ago
|
||
Managed to reproduce the issue with older builds such as 25.0a1/32.0a1 builds before this don't seem to load the page at all.
It seems this is not a regression and as per comment 1, this would be an issue with Outlook.
Comment 6•4 years ago
|
||
This issue was reproducible on latest Nightly version 74.0a1 (2020-02-04) on Windows 10
Comment 7•4 years ago
|
||
This issue was reproducible on Latest Nightly version 75.0a1 (2020-03-02) on Windows 10. Changed flag accordingly
Comment 8•4 years ago
|
||
This issue was reproducible on Latest Nightly version 76.0a1 (2020-03-18) (64-bit) on Windows 10. Changed flag accordingly
Comment 9•4 years ago
|
||
This issue is reproducible on Windows 10 with Firefox Nightly version 77.0a1 (2020-04-15) (64-bit)
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D77027
Assignee | ||
Comment 12•4 years ago
|
||
This patch adds the following constraints:
- close the dialog when the originating context navigates;
- close the dialog when the originating context closes / goes away;
- do not allow more than 1 dialog at a time;
- do not allow opening the dialog from an inactive (background) browser.
Depends on D77028
Comment 13•4 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ecef5e940485 pass the triggering principal when opening external URIs, r=ckerschb https://hg.mozilla.org/integration/autoland/rev/47c229951c8b do not allow navigating to external URIs in cross-origin disjoint browsing contexts, r=johannh,smaug https://hg.mozilla.org/integration/autoland/rev/b4b26ce1c20e restrict opening the 'open external protocol' dialog, r=johannh,fluent-reviewers,flod
Comment 14•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ecef5e940485
https://hg.mozilla.org/mozilla-central/rev/47c229951c8b
https://hg.mozilla.org/mozilla-central/rev/b4b26ce1c20e
Comment 15•4 years ago
|
||
Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.
Updated•4 years ago
|
Comment 16•4 years ago
|
||
The patch landed in nightly and beta is affected.
:Gijs, is this bug important enough to require an uplift?
If not please set status_beta
to wontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 17•4 years ago
|
||
Assignee | ||
Comment 18•4 years ago
|
||
Comment on attachment 9156454 [details] [diff] [review]
Beta patches for protocol restrictions
Beta/Release Uplift Approval Request
- User impact if declined: Confusing / DoS-style issues with the external protocol dialog
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: n/a
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): Medium because it's not that early in the cycle anymore. However, the patches are reasonably straightforward, have some automated test coverage, and they fix a few security/DoS style issues with external protocols, so I think they're worth taking. There's also an about:config pref we can use to disable the aspect of this patch with the most risk (disallowing external URLs opened on cross-origin popups).
- String changes made/needed: not in this patch for uplift
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: DoS-style (sec-low/sec-vector) security issues prior to the release of the first ESR
- User impact if declined: (as above)
- Fix Landed on Version: 79
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): (as above)
- String or UUID changes made by this patch: not in this patch for uplift
Comment 19•4 years ago
|
||
Maybe we can let this ride to 79 and get the fix in ESR for 78.1?
Assignee | ||
Comment 20•4 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #19)
Maybe we can let this ride to 79 and get the fix in ESR for 78.1?
I'm not worried about landing this on beta at this point in the cycle. However, iIf enterprise things use external protocol handlers, this change may affect them, and landing it on ESR after shipping the initial ESR release seems more risky (to them) than landing it before we ship the first one.
Comment 21•4 years ago
|
||
Comment on attachment 9156454 [details] [diff] [review] Beta patches for protocol restrictions alright then, approved for 78.0b8, it'll go into esr78 from there.
Comment 22•4 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/28b5472e0d82
https://hg.mozilla.org/releases/mozilla-beta/rev/c3f184d5cba9
https://hg.mozilla.org/releases/mozilla-beta/rev/5de3646dd424
Comment 23•4 years ago
|
||
Backed out for ESLint failures (and I'm guessing eventually other test failures too given the nature of the reported issue).
https://hg.mozilla.org/releases/mozilla-beta/rev/bdb9a8c9c312748fd764664e3808d2c0e1f4e2d3
Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306526838&repo=mozilla-beta&lineNumber=124
Assignee | ||
Comment 24•4 years ago
|
||
Sorry, unused variable left after removing the string changes, but there shouldn't have been any ill effects from that other than the eslint failure...
Comment 25•4 years ago
|
||
Comment on attachment 9157081 [details] [diff] [review] Beta patches for protocol restrictions Re-approved for 78.0b8. Thanks for the fix-up.
Comment 26•4 years ago
|
||
bugherder uplift |
Description
•