-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require user gestures to accept registerProtocolHandler request #7797
Comments
Yeah, that sounds good to me. cc @gijsk |
I am curious why you believe this? I don't think we do... cf. https://bugzilla.mozilla.org/show_bug.cgi?id=668577 and all the complaints from office365 users when they started calling |
Umm. I see. I didn't manage to create a test that avoids the prompting dialog to complete the registration. I guess I assumed wrongly that it was a requirement, since Chromium has code to explicitly ignore it. I see that the "do not ask again" may have sense as well, so the question is whether we want to clarify the expected behavior regarding how the request is initiated or not. This issue might be relevant to define our strategy for implementing WPT for the registerProtocolHandler method, as it was mentioned in issuecomment-742475137 Additionally, I wonder whether this could be a source of interoperability issues. |
For sure. I mean, without wanting to speculate on the development practices of the Office 365 folks, it seems plausible that they tested things in Chrome/Edge and didn't realize that they had some rPH calls that were effectively no'opd, which would cause issues in other browsers. (We shipped a site-specific change to remediate the user-facing issue on the Office site.) |
I guess there are three cases: (1) User has not been asked about the protocol already, we can require registerProtocolHandler to be instantiated by a user gesture in order to prevent annoying registration. (2) User has approved the registration of a protocol for a given page handler, in that case we probably don't need a user gesture anymore (what Chromium does I think). (3) User has already rejected registration, in that case requiring a user gesture does not really make a difference since registration will eventually fail in any case. |
(2) manifests when you want to update the handler URL but nothing else? Seems reasonable given that you're not supposed to be able to determine success anyway. (Although implementations have bugs here, so...) |
I see, thanks @gijsk for the feedback. So I guess the next step is to figure out how detailed we want this user gesture restrictions to be defined in the spec. If I'm not wrong the only statement defined on the spec on this regard is the following:
I think it'd be reasonably to specify clearly the cases where is mandatory to prompt the user (aka, user gesture requirement). There shouldn't be too many, and I guess we can get an agreement here between at least 2 implementors. Regarding case (2) described in this comment, I think Chromium manages silently the update operation, not sure about FF.
It'd be useful, although perhaps not ideal for a spec, to have some examples of cases where this silent registration may happen. Finally, there is this statement in the spec that I think it's very relevant for the "don't ask me again" :
I think we can clarify here that is user's decision to opt-out of these confirmation prompts. Also, as far as I know, neither Chromium or Firefox implement this part of the spec. |
The Custom Handlers spec [1] stats the following: "A user agent could, for instance, prompt the user and offer the user the opportunity to add the site to a shortlist of handlers, or make the handlers their default, or cancel the request." Chrome already implements this in the Browser::RegisterProtocolHandler function, checking the user_gesture parameter; the value of this parameter is determined by the renderer process in the NavigatorContentUtilsClient function, checking the UserActivation of the source frame. Chrome ignores (don't store the handler in the registry) request issued without an user gesture. The handler will be passed to the PageSepecificContentSettingsDelegate so that it's hold as pending. The user may later confirm the registration via the content settings page, ensuring the process is started by an user gesture. This CL provides new browser tests for chrome to verify the user gesture requirement. It's worth mentioning that this behavior was introduced long ago in r139996 (see the CL [2] for details) in response of several bug reports filed by end users. We've added this requirement, despite the spec, as described above, is quite vague regarding it. We are currently discussing in a WATHWG issue [3] whether to make the user gesture requirement more strict and clearer in the spec. There are some cases (eg, testing) where we may want to avoid prompting the user; the spec also states something on this regard_ " ... user is not repeatedly prompted with the same request." There is also a bug report in Firefox to offer users a "do not ask again" option in the prompt dialog. Finally, this CL implements the user_gesture requirement on the implementation of the registerProtocolHandler method we have for the ContentShell. We've landed this implementation recently in r988671 with testing purpose in mind, WPT especially where we lack tools to properly simulate the user gesture to issue the registration request. However, the shell is not used exclusively for testing, so I consider important that it matches chrome on this issue. The CL also provides browser tests for the Custom Handlers component, as I've done for chrome. However, since there is no content-settings page for the shell, the tests is slightly different. Particularly, it's worth mentioning that it relies on the registry's OnDenyRegisterProtocolHandler function, which actually stores the handler in the registry, to inform the observers defined in the tests that the request has been completed. [1] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers [2] https://source.chromium.org/chromium/chromium/src/+/3a3b75ab9cc13b9acb466de0ef1bc7036756fce4 [3] whatwg/html#7797 Change-Id: Ic6e2e449eabee9d2eded7c20d939fd9ebc46ba5a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572056 Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Cr-Commit-Position: refs/heads/main@{#991220}
To be more precise, it seems that Chrome requires that the registerProtocolHandler is instantiated by a frame the has a Transient Activation. Independently of the issue of whether to be more precise describing when and how to prompt the user, I think the spec should describe explicitly the relationship between the requesterProtocolHandler and the Tracking User Activation section. |
Would be a valid solution to throw an exception if the registerProtocolHandler request has been instantiated without any user gesture ? That would make easier to define WPT for this requirement; assuming we agreed on add it explicitly to the spec. We could, for instance, use test_driver.bless in a similar way than in the show-picker-user-gesture.html test. Looking at the available DomException error names we could, perhaps, use InvalidStateError; I guess NotAllowedError would perhaps fit better on this use case, but it doesn't have a code name and value, which is useful to define the asserts in the tests. |
I would prefer ignoring as that would more likely be web-compatible, even if that ends up being harder to test. Adding exceptions to an API is generally fraught with problems. |
Chrome ignores registerProtocolHandler requests if they are not issued by user gestures. In chrome, this is evaluated based on the frame's UserActivation state.
I believe that Firefox has the same requirement.
I couldn't find in the custom handlers spec any reference to this requirement.
Shouldn't we add it explicitly ? It seems there are good reasons (eg confusing user experience; see bugs 119438, 62799 and 724919 for details)
The text was updated successfully, but these errors were encountered: