Skip to content
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

Add test API for protocol handlers #26819

Closed
fred-wang opened this issue Dec 9, 2020 · 21 comments
Closed

Add test API for protocol handlers #26819

fred-wang opened this issue Dec 9, 2020 · 21 comments

Comments

@fred-wang
Copy link
Contributor

cc @annevk

In #23504, several manual tests were added

https://github.com/web-platform-tests/wpt/tree/master/html/webappapis/system-state-and-capabilities/the-navigator-object/

because we lacked a proper API to emulate user's approval of a protocol handler registration.

When fixing the bug in Chromium https://chromium-review.googlesource.com/c/chromium/src/+/2379672 , I had to rely on C++ browser tests that auto-approve registration. It would be good if such a logic were in WPT instead.

@annevk
Copy link
Member

annevk commented Dec 10, 2020

Makes sense, do we just need a switch for auto-granting the permission? I'd be happy to do the WebDriver specification part of that in HTML.

@fred-wang
Copy link
Contributor Author

Thanks Anne.

I opened this for the record and I have not really investigated into this to be honest, but checking the chromium tests, I see 3 things:

  • something to claim registerProtocolHandler calls are initiated by a user interaction (as otherwise it will get rejected).
  • something to force auto-granting permission to bypass the UI.
  • some callback / promise to ensure the protocol has been registered and we can just go ahead testing it.

(I also wonder whether we want something more generic than just protocol handler. A quick search https://source.chromium.org/search?q=set_auto_response_for_test shows matches for WebVR or WebRTC for example)

@annevk
Copy link
Member

annevk commented Dec 10, 2020

I think there is a generic permission thing we could use. User interaction can already be done through testdriver.js (i.e., simulating a mouse click or some such). The success callback seems trickier though. Maybe @jgraham and @stephenmcgruer have ideas.

@fred-wang
Copy link
Contributor Author

OK, thanks.

For the record, why the success is not web-exposed is explained in whatwg/html#5845

@jgraham
Copy link
Contributor

jgraham commented Dec 10, 2020

Yeah, so the way I'd imagine this working is that you first grant the permission in general, then synthesise a user guesture that causes an event that runs the registerProtocolHandler code. If there are steps after that which are asynchronous or state that we want to expose to tests but not to the web things do get more tricky. We could image a test-only DOM API allowing us to inspect the registered protocol handlers, and poll that API to check for success. Maybe we could have a test-only event, but that feels pretty exotic. I"m not sure if there's a way we could express this as a HTTP WebDriver endpoint (maybe this is a case where WebDriver-BiDi will eventually help since we could expose the registerProtocolHandler result as an event in that API without exposing it to the DOM).

@annevk
Copy link
Member

annevk commented Dec 10, 2020

Maybe a "reply once X is registered" endpoint, essentially a hanging GET. Seems a bit nicer than having to poll, but I suppose either works. It might be worth waiting a bit as well though for the better infrastructure to arrive.

chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug : 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
@javifernandez
Copy link
Contributor

@annevk who could be a good candidate to look at the #33866 PR ? It's an automatic PR crated by the CL I m working on to add support in Chrome for the new PermissionDescriptor, so that we can effectively grant or deny registerProtocolHandler request without user interaction.

I hope this is also supported by Firefox, or at least assume it has interest to do so otherwise.

@annevk
Copy link
Member

annevk commented Apr 29, 2022

Maybe @jgraham? To be clear, this is not a permission that is exposed to web content, right? Just Test Driver?

chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue Apr 29, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue May 3, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue May 4, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue May 6, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
@javifernandez
Copy link
Contributor

javifernandez commented May 6, 2022

Maybe @jgraham? To be clear, this is not a permission that is exposed to web content, right? Just Test Driver?

Well, the change I'm doing in Chrome would indeed allow to perform queries like this:

      navigator.permissions.query({ 'name': 'protocol-handler' } )
         .then(function(value) {
             done({ status: 'success', value: value && value.state });
           }, function(error) {
             done({ status: 'error', value: error && error.message });
           });

returning a success` status and the corresponding permission's value.

chromium-wpt-export-bot pushed a commit that referenced this issue May 9, 2022
The idea of this CL is to define a new mapping between blinks's
PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing
discussion in with the WPT community in the issue #26819 [3], where it
seems to be enough agreement on the idea of using the webdriver
setPermission API to grant or deny permission for the
registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
@annevk
Copy link
Member

annevk commented May 10, 2022

That doesn't sound great? Where is the proposal for that permission as well as its standardization venue?

chromium-wpt-export-bot pushed a commit that referenced this issue May 10, 2022
The idea of this CL is to define a new mapping between blinks's PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing discussion with the WPT community in the issue #26819 [3], where it seems to be enough agreement on the idea of using the WebDriver setPermission API to grant or deny permission for the registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue May 10, 2022
After we landed in rXXXX the new protocol-handler permission descriptor we can now use it to set the permission status in WPT, using WebDriver.

This CL changes the registerProtocolHandler method implementation of both the content-shell and Chrome to check whether a permission status has been already set. In content-shell we don't ask the user, but in chrome we use the RequestPermissionManager, which eventually may launch a prompt dialog.

We only have manual tests for the registerProtocolHandler method, precisely due to the user_gesture requirement and the prompt dialog. We
can now use the Permission interface, via the extension commands [1] that are implemented by the WebDriver API.

In the case of the content-shell, we are just looking for any permission status already set and in the WebTestPermissionManager instance. For Chrome, instead, we have added a new method to check the  devtools_permission_overrides_ cache. We want in this case to avoid getting the actual permission status (using the helper).

There is an ongoing discussion [2] with the WPT community in the issue in this CL.

[1] https://w3c.github.io/permissions/#automation
[2] #26819

Bug: 1321073
Change-Id: I4fb9cd0871fb9d086071a151cdea7642dbcd3e2b
@javifernandez
Copy link
Contributor

javifernandez commented May 10, 2022

That doesn't sound great? Where is the proposal for that permission as well as its standardization venue?

This is still an idea, nothing has been landed yet. I'd appreciate any suggestion on how to build a proposal for standardization. I'd simply tried to implement what it's been suggested in this comment regarding the "something to force auto-granting permission to bypass the UI." issue.

I think there is a generic permission thing we could use

The idea is to use the SetPermission extension commands for the [WebDriver] specification for Automated testing, defined in the Permission specification.

In order to to use the command we need to use a PermissionDescriptor, which can be used to retrieve a permission status using the query method of the Permission Interface.

I've got some inspiration on the tests we already have for the clipboard-apis, like the readText-granted.https.html test, where we are using the set_permission WebDriver command to grant permission associated to the "clipboard-read" descriptor.

I guess we would need to add a Permission API integration section for the Custom Handlers spec, like the one defined in the Clipboad API spec.

I'm not sure where is the right standardization venue for the PermissionDescriptor name. I've found that Firefox defines the Permissions.webidl while Chrome has the permission_descriptor.idl both diverging quite a lot in the enumeration of permission names, although both refer to https://w3c.github.io/permissions/#enumdef-permissionname which doesn't seem to exist anymore.

Not sure if this explain the divergences between Firefox and Chrome regarding the PermissionDescriptor name, but the spec has a note in the query method that states the following:

This is deliberately designed to work the same as WebIDL's enumeration (enum) and implementers are encouraged to use their own custom enum here. The reason this is not an enum in the specification is that browsers vary greatly in the powerful features they support. Using a DOMString to identify a powerful feature gives implementers the freedom to pick and choose which of the powerful features from the Permissions Registry they wish to support.

chromium-wpt-export-bot pushed a commit that referenced this issue May 11, 2022
The idea of this CL is to define a new mapping between blinks's PermissionType enum and the PermissionDescriptor structure defined in by the Permission [1] interface.

The new mapping will be useful to provide permission automation for the
testing of the CustomHandler HTML API [2]. There is an ongoing discussion with the WPT community in the issue #26819 [3], where it seems to be enough agreement on the idea of using the WebDriver setPermission API to grant or deny permission for the registerProtocolHandler method, avoiding the user interaction.

This CL tries to cover all the end points of the Permission interface,
either the InternalPermission class or through the CRDTP.

[1] https://w3c.github.io/permissions/#permissions-interface
[2] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
[3] #26819

Bug: 1321073

Change-Id: I289bc89bd1be31b18fd91ed3d81c6dd5fe853a0c
chromium-wpt-export-bot pushed a commit that referenced this issue May 11, 2022
After we landed in rXXXX the new protocol-handler permission descriptor we can now use it to set the permission status in WPT, using WebDriver.

This CL changes the registerProtocolHandler method implementation of both the content-shell and Chrome to check whether a permission status has been already set. In content-shell we don't ask the user, but in chrome we use the RequestPermissionManager, which eventually may launch a prompt dialog.

We only have manual tests for the registerProtocolHandler method, precisely due to the user_gesture requirement and the prompt dialog. We
can now use the Permission interface, via the extension commands [1] that are implemented by the WebDriver API.

In the case of the content-shell, we are just looking for any permission status already set and in the WebTestPermissionManager instance. For Chrome, instead, we have added a new method to check the  devtools_permission_overrides_ cache. We want in this case to avoid getting the actual permission status (using the helper).

There is an ongoing discussion [2] with the WPT community in the issue in this CL.

[1] https://w3c.github.io/permissions/#automation
[2] #26819

Bug: 1321073
Change-Id: I4fb9cd0871fb9d086071a151cdea7642dbcd3e2b
chromium-wpt-export-bot pushed a commit that referenced this issue May 11, 2022
After we landed in rXXXX the new protocol-handler permission descriptor we can now use it to set the permission status in WPT, using WebDriver.

This CL changes the registerProtocolHandler method implementation of both the content-shell and Chrome to check whether a permission status has been already set. In content-shell we don't ask the user, but in chrome we use the RequestPermissionManager, which eventually may launch a prompt dialog.

We only have manual tests for the registerProtocolHandler method, precisely due to the user_gesture requirement and the prompt dialog. We
can now use the Permission interface, via the extension commands [1] that are implemented by the WebDriver API.

In the case of the content-shell, we are just looking for any permission status already set and in the WebTestPermissionManager instance. For Chrome, instead, we have added a new method to check the  devtools_permission_overrides_ cache. We want in this case to avoid getting the actual permission status (using the helper).

There is an ongoing discussion [2] with the WPT community in the issue in this CL.

[1] https://w3c.github.io/permissions/#automation
[2] #26819

Bug: 1321073
Change-Id: I4fb9cd0871fb9d086071a151cdea7642dbcd3e2b
@javifernandez
Copy link
Contributor

I've created an issue in the html spec to discuss the standardization of this new permission name.

@annevk
Copy link
Member

annevk commented May 13, 2022

What I was hoping is that we could have something that does not end up being web-exposed.

@johannhof
Copy link
Member

I strongly agree with Anne, this doesn't feel like it should be web-exposed. I get the idea of integrating with permissions properly but you're really adding a lot of complexity through exposure that is not needed for this, so I would recommend whatever hacky alternative exists :)

@javifernandez
Copy link
Contributor

javifernandez commented May 17, 2022

I understand there is strong opposition to introduce a new permission for the Custom Handler spec, fair enough. Having said that, I'm still interested on pursuing what it was the main goal of this issue: Testing Automation. Is this still something worth pursuing ? If that's the case, I'd appreciate some guidance to address the following issues.

@annevk What did you have in mind in the comment above when you said "I think there is a generic permission thing we could use. " ? Wasn't the idea to use the Set Permission command ?

What I was hoping is that we could have something that does not end up being web-exposed.

I admit that it was my intention when I started to work on this. I assumed (maybe wrongly ?) that the idea was to use the Set Permission command and since it requires a PermissionSetParameters, which uses a PermissionDescriptor as one of the inputs, I thought it was needed to define a new permission name for "custom handlers". I then realized that such new descriptor will be exposed through the query; it's then when I though that perhaps it'd be interesting to provide a new section to the Custom Handler spec to integrate it with the Permission API (that's what I proposed in issue 7920). As I said, it seems clearly there is no support for it, lets forget and move on.

If the idea was to use "Set Permission" how we could avoid the web-exposure issue ?

Is there any other way to implement this "auto-granted" permission behavior for WPT ? I know ways to do this in Chome, using the InternalsPermission interface, but that would mean that the tests will fail when executed by the WPT runner; I'd rather avoid that scenario.

@annevk
Copy link
Member

annevk commented May 19, 2022

I recommend reaching out to @jgraham on Matrix.

@javifernandez
Copy link
Contributor

If I've understood it correctly, @jgraham propose the definition of a new WebDrviver endpoint to handler the permission prompt. I also have understood that the ideal approach would be to use the new WebDriver-BiDi but unfortunately this won't be available soon. Regarding the new endpoint, I've filed w3c/webdriver#1677; it seems that this user prompts related issue is affecting other testing use cases, so perhaps it'll be closed as duplicated.

Anyway, while studying the WebDriver spec I found out that there is indeed a way of handling simple prompt dialogs. Wouldn't be possible to use one of these as part of the registerProtocolHandler method ?

The spec doesn't mention at all which kind of dialog should be used; it states.

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. User agents could also silently collect the information, providing it only when relevant to the user.

Wouldn't be possible to change to text in the spec to suggest the use of a specific dialog ?

@javifernandez
Copy link
Contributor

Another approach, more in the line of the original idea to define a new WebDriver endpoint, would be to define an 'extension command' similar to what we have for the Secure Payment Confirmation feature with the Set SPC Transaction Mode command.

@javifernandez
Copy link
Contributor

At lest @foolip expressed interest on defining a new WebDriver endpoint for the Custom Handlers, so that we could define some kind of autoreply on the prompt dialog and define automated tests for the registerProtocolHandlers method.

I've filed an issue for the html spec to discuss this idea and figure out whether there is enough support from at least 2 implementers, so that a PR for the spec is worth. It'd be great to have some feedback from Mozilla, perhaps @jgraham ?

@javifernandez
Copy link
Contributor

We have now a new WebDriver extension command to avoid the user prompt dialog, which together with the bless API, should be enough to provide testing automation for protocol handlers.

I'm currently working on the implementation for this command for chromedriver and it seems there is "some" interest expressed (see here and here) by Firefox. I have the PR#35792 for automated tests covering the cases we already have in the manual tests.

@javifernandez
Copy link
Contributor

javifernandez commented Jun 5, 2024

This is implemented in Chrome [1] and defined the proper APIs and actions in the WPT scripts [2], so I think we can close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants