New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 724919 link

Starred by 38 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Ignore the "Remember my choice" checkbox when the user chooses to not launch an external protocol link

Project Member Reported by dominickn@chromium.org, May 22 2017

Issue description

In M57, we added a metric to record whether or not users checked the "Remember my choice" checkbox in the external protocol handler when they responded to the dialog.

The metric results are at https://uma.googleplex.com/p/chrome/timeline_v2/?sid=88c7c2d6b0949da518d288e4ed56227b. In summary:

Launch protocol, don't remember: 86% 
Don't launch protocol, don't remember: 9.4%
Launch protocol, remember: 4.4%
Don't launch protocol, remember: 0.2%


Based on these results, I would be comfortable changing the text of the checkbox to read "Always launch links for <app name>", and ignore it when the user decides to not launch the protocol.

This will help stop users from getting in the bad state of permanently not being able to launch external protocols. It also removes an interaction that only 0.2% of users end up selecting.

What are people's thoughts on this? This is orthogonal to adding new settings UI for the protocol handlers.
 
Sounds great to me! :-D
Anyone have strong objections to this proposal? Otherwise we should start implementing shortly.
Cc: srahim@chromium.org
I don't feel strongly but we are taking away some user agency which is unfortunate.  I'd prefer to just add the settings UI, but since that isn't likely to happen I think we should view this as a temporary change until we get proper settings UI.

BTW the 0.2% could also be used to show this isn't actually that big of a problem.

Regardless, if we make the change we should review the strings.
Cc: mgiuca@chromium.org
+mgiuca.

We're getting some noise again on  Issue 62799 , so it would be good to come to a decision on this.
Cc: -mgiuca@chromium.org
Owner: mgiuca@chromium.org
Status: Assigned (was: Available)

Comment 6 by srahim@chromium.org, Jul 24 2017

Dominick and Ben, did we consider removing the checkbox entirely? Looking at 671658#c27, ainslie@ recommended removing the checkbox (though he did not block the feature if the checkbox was kept). The metric shows that 95% of users do not opt to "always open links for <app>">.  

Removing the checkbox entirely could be another option, though that may annoy frequent users of external protocol links since they will need to confirm every single launch.
My recommendation is to remember only the affirmative case. So frequent users who want the link opened in other apps will not be affected as they can remember the choice, but the destructive (ie, task flow no longer works) and hard to recover from choice cannot be remembered.
Cc: mgiuca@chromium.org
Owner: benwells@chromium.org
Let's go with only applying the checkbox int he affirmative case, as pinkerton@ and dominickn@ suggest.

srahim - are you comfortable with the proposal as is? I.e. change the text of the checkbox to "Always launch links for <xxx>."?
WRT the string, I actually think "Always launch links for Facebook" could be improved. It's not links that are launching; it's the app. I was thinking of something more like "Always open Facebook links in the app". 
Dom, Mike - are you happy with "Always open Facebook links in app"?
I think the <xxx> in comment #9 is the macOS app name, not the name of the site, so there's a disconnect between comment #9 and comment #10. I don't expect us to ever say "facebook links", because that's not the thing that's actually being opened (it would be an iTMS link), even if it happens to be on facebook. Furthermore, it isn't specific to any one website. 
"Always open <app name> links in the app" SGTM.

c#12: in the case of iTMS links, it would say "Always open iTunes links in the app", which seems fine to me?
I think Facebook is a bad example. Re #12: yes, <xxx> was meant to be the app name.

Ugh, this is more complicated than I thought. The way the system works, what is remembered is whether the scheme of the links should be opened in whatever the registered application is. If the registered app changes, the persisted choice still applies.

Magnet links are a great example where multiple apps can open them. E.g., this could happen:

1. User installs a magnet handler (e.g. uTorrent)
2. User clicks on a magnet link.
3. Chrome asks if the user wants to open it in uTorrent, with the checkbox.
4. User checks box and accepts
5. User installs a different magnet handler (e.g. EvilTorrent)
6. User clicks on another magnet link
7. At this point Chrome will automatically launch EvilTorrent.

Given this, the checkbox in 3. should not be talking about uTorrent. It should just be talking about the type of link. Or, we should change the system to remember the app and reset the setting if the app changes.

I can't see us changing the system in the short term. Given that, how about we change the string to not mention the app name.

New suggestion: "Always open these types of links."
Or: "Always open magnet links." / "Always open itms-app links."

Thoughts?
I just remembered the current string has already gone through many rounds of discussion in  issue 601725 . Despite that, I think it is currently misleading, and that wasn't considered on the previous discussion. Sorry to stir it all back up....

emilyschechter - you were a big participant of the original discussion, what do you think of #14?
Since Ben asked me to weigh in:

1. Yes, I agree with the initial premise, the behavioural change that the checkbox to remember my choice should *not* take effect if the user clicks Cancel. This is a basic principle of a dialog that a "Cancel" button should not have any side effect, and remembering that choice permanently is a side effect.
2. The discussion then is about what string to use on the checkbox. I agree with #14 that we should not mention the name of the app itself because that can change after the choice has been remembered. However, "always open these types of links" or similar is unclear (what does "open" mean... open where?) I do like explicitly mentioning the link scheme in the text.

So I would propose text like "Always open magnet links in another application".
c#14: good catch here.

c#16: just using the scheme is going to look weird when schemes and app names overlap, e.g. skype:// or facetime:// links will be displayed lowercase:

Always open skype links in another application
Always open facetime links in another application

Seeing "skype"/"facetime" seems weird to me when they're meant to be capitalised, and seeing "itms" is even worse.

"Always open these links in another application" or something along those lines feels better to me since it'll avoid having odd looking strings when schemes refer directly to an app name or brand.
I tend to agree, but I would prefer to show those scheme names. Perhaps adding more syntax makes it clear this refers to the scheme not the brand: "Always open "skype:" links".

I'm not fussed either way.
(Let's not forget that even though we think of URLs as an implementation detail that user's don't care about, we *are* surfacing it to the user in the link hover text, so it is still part of the user interface and we can refer to parts of the URL in our messages.)
Shimi - any thoughts?
Cc: ainslie@chromium.org
+ainslie for recommendation on if we should be showing the scheme (which is nonsensical to most, but expert users will understand) in the message text. 

Can we get a mockup of the full UI of the dialog? We're so busy going back and forth about the wording of the checkbox, I can't even recall the primary dialog text presented to the user. 
+1 for proposed behavioral change (c#9).

For the string, IMO showing scheme like "skype:" is likely to make normal users glaze over the dialog, and people won't really understand that "skype:" is a category that means scheme vs. brand. I would prefer to use something less syntaxy.

Variations:
"Always open uTorrent links, and similar links of this type, in that app."
"Always open uTorrent links in the app. If you install a different app to open these type of links, always open in that app."
"Always open uTorrent links in the app. If you install a different app to open these type of links, links will open in that app."
Owner: srahim@chromium.org
Assigning to Shimi to figure out the strings.

Mike - screenshot attached.
Just realised if we show the app name (as a friendly name for the protocol), you get some bad edge cases. e.g., if the string is "Always open $APP_NAME links in that app.", there are two issues:

1. If you have a generic protocol like "mailto", it will say "Always open Outlook links in that app." But it's about generic mail links, not Outlook. (Having said that, I think we have a special string for well-known protocols, so this may not be a common issue?)
2 [more concerning]. If you have an app that handles two protocols, say that iTunes handles both "itunes:" and "appstore:" hypothetically; it will say "Always open iTunes links in that app." But that's a lie because it's only going to set the policy for one of those protocols.

There's an extreme example of #2 on Linux where all links open in an app called "xdg-open", and the dialog reads "Remember my choice for xdg-open links"; what it actually means is "Remember my choice for <the current protocol> links". The xdg-open issue is a separate problem, but it's an example of why it's bad to use the app title when you really mean the protocol.
Would the following string work:

"Always open these types of links in the associated app"

This would avoid showing the app name, which could change, and the scheme, which most users won't understand. But it still lets the user know that links like they one they clicked will open an app, not a page on a website in their browser.    

Screenshots are in this associated bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601725
That SGTM, Shimi.
Important tip for anyone who fixes this: I just discovered that we blacklist certain protocols (like "shell" and "vbscript") for security, using the very mechanism that is proposed to be deleted here!

https://cs.chromium.org/chromium/src/chrome/browser/external_protocol/external_protocol_handler.cc

e.g., "vbscript" is not hard-coded as a blacklisted protocol, but rather, it is inserted into the default prefs file as a protocol as if the user ticked "Remember my choice" and then pressed "Don't open".

So if you just change the behaviour in the dialog box, this is safe, but if you actually remove the remembering of choices, you may inadvertently allow execution of bad things, so be careful.
Owner: benwells@chromium.org
Status: Started (was: Assigned)
As part of this change the cancel text is changing from "Don't open" to "Cancel". Also, the default button is changing from Cancel to the open button.

Shimi / Emily - do you think this makes sense?
BTW I'm quite unsure about changing the default button. It seemed weird to have a default cancel button, but I'm also worried people might accidentally open things in external applications with the ok button as default.

So - feedback appreciated!
Here is a screenshot from Linux. Will post from Mac soon.
Screenshot from 2017-08-31 13:51:46.png
15.7 KB View Download
Here is the Mac screenshot.
Screen Shot 2017-08-31 at 2.30.40 pm.png
151 KB View Download
Project Member

Comment 32 by bugdroid1@chromium.org, Sep 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f227110f19e30a20509488005f8c6cb29f157833

commit f227110f19e30a20509488005f8c6cb29f157833
Author: Ben Wells <benwells@chromium.org>
Date: Wed Sep 06 03:07:56 2017

Ignore remember checkbox in external protocol dialog when user cancels.

As part of this change the text of the checkbox is also changing to
"Always open these types of links in the associated app", the text of
the cancel button is changing from "don't open" to "cancel", and the
default button is changing from the cancel button to the ok button.

Bug:  724919 
Change-Id: Iff02a20394d78451be4832318eee329f92f714c0
Reviewed-on: https://chromium-review.googlesource.com/644517
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499865}
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/app/generated_resources.grd
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/chromeos/external_protocol_dialog.cc
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/chromeos/external_protocol_dialog.h
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/external_protocol/external_protocol_handler.cc
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/external_protocol/external_protocol_handler.h
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/cocoa/external_protocol_dialog_cocoa.mm
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/external_protocol_dialog_delegate.cc
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/external_protocol_dialog_delegate.h
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/protocol_dialog_delegate.h
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/views/external_protocol_dialog.cc
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/views/external_protocol_dialog.h
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc
[modify] https://crrev.com/f227110f19e30a20509488005f8c6cb29f157833/tools/metrics/histograms/histograms.xml

Cc: owe...@chromium.org sa...@chromium.org rnimmagadda@chromium.org ajha@chromium.org windr...@google.com
 Issue 62799  has been merged into this issue.
Project Member

Comment 34 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07

commit cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07
Author: Ben Wells <benwells@chromium.org>
Date: Thu Sep 21 06:57:57 2017

Ignore any saved blocked external protocol schemes.

External protocol schemes cannot be blocked any more via the UI, however
past decisions to block schemes could be saved. This change ignores any
persisted block decisions except for the hard-coded list of schemes that
should always be blocked.

The change also simplifies the way hard coded schemes are treated and
removes some migration logic that is no longer.

Bug:  724919 
TBR: msramek@chromium.org
Change-Id: Id39fb759b035182a1131e69e90ad0a3e1a425b65
Reviewed-on: https://chromium-review.googlesource.com/674469
Commit-Queue: Ben Wells <benwells@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503375}
[modify] https://crrev.com/cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07/chrome/browser/external_protocol/external_protocol_handler.cc
[modify] https://crrev.com/cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07/chrome/browser/external_protocol/external_protocol_handler.h
[modify] https://crrev.com/cafcc8289fbb61ab44bfaa3e9e8e7c05cab98b07/chrome/browser/external_protocol/external_protocol_handler_unittest.cc

Status: Fixed (was: Started)
Labels: -Restrict-View-Google
Not sure why this was RVG.

Sign in to add a comment