w3c / webdriver

Remote control interface that enables introspection and control of user agents.
https://w3c.github.io/webdriver/
Other
679 stars 194 forks source link

Add a capability for returning the default user agent header value #1790

Closed OrKoN closed 6 months ago

OrKoN commented 7 months ago

The capability cannot be used for matching as it only provides a computed value.

Issue https://github.com/w3c/webdriver-bidi/issues/446 Related https://github.com/w3c/webdriver-bidi/pull/652


Preview | Diff

whimboo commented 7 months ago

This looks fine to me. @jgraham and @gsnedders mind checking as well?

gsnedders commented 7 months ago

Does this not also need processing in "validate capabilities" (and maybe more in "matching capabilities"? it's not clear to me whether that's deliberately only a subset of capabilities).

As far as I can tell, with this PR passing:

{
  "capabilities": {
    "alwaysMatch": {
      "userAgent": "This/1.0 Is/2.0 Not/3.0 (A User-Agent String"
    }
  }
}

…does nothing, and it will still manage to satisfy the capabilities?

OrKoN commented 7 months ago

@gsnedders I intended to add userAgent as an output capability only as described by this definition("provide any computed value to return to the user") so it should not be interpreted as part of the capabilities request (and my understanding is that in this case updates to the validation algorithms are not needed).

whimboo commented 7 months ago

Note that no-one will most likely match capabilities on the user agent string given its wide variance. Therefore we have the browserName and platform instead.

OrKoN commented 7 months ago

So I believe the following in the request

{
  "capabilities": {
    "alwaysMatch": {
      "userAgent": "This/1.0 Is/2.0 Not/3.0 (A User-Agent String"
    }
  }
}

would result in a failure to create a session according to https://w3c.github.io/webdriver/#dfn-matching-capabilities (Step 3, the Otherwise clause).

We had a discussion with @sadym-chromium and @jrandolf and we are wondering if we should instead allow clients to opt-in into getting the capability result? E.g., by sending

{
  "capabilities": {
    "alwaysMatch": {
      "userAgent": true
    }
  }
}

the client will receive the userAgent string in the response.

OrKoN commented 7 months ago

Updated the PR to handle userAgent when matching capabilities.

jgraham commented 7 months ago

FWIW I'm happy with this only being returned, not used as the basis for a match.

whimboo commented 7 months ago

@OrKoN I assume that you want to add a new test for classic that checks for the user agent to be part of the session capabilities? We should probably have a separate test for now until all browsers have support for it.

whimboo commented 6 months ago

@OrKoN are you going to add a wpt test for it?

OrKoN commented 6 months ago

Yes, I am on it