search

w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
375 stars 42 forks source link

(possibly) Conflicting definitions for ScriptResult type #217

Closed juliandescottes closed 2 years ago

juliandescottes commented 2 years ago

I found two "definitions" for script's ScriptResult. One at 7.3.1 https://w3c.github.io/webdriver-bidi/#module-script-definition

ScriptResult = (
  ScriptCallFunctionResult //
  ScriptEvaluateResult //
  ScriptGetRealmsResult
)

and another at 7.3.2.6 https://w3c.github.io/webdriver-bidi/#type-script-Result

ScriptResult = (
  ScriptResultSuccess //
  ScriptResultException
)

ScriptResultSuccess = {
  result: RemoteValue,
  realm: Realm
}

ScriptResultException = {
  exceptionDetails: ExceptionDetails
  realm: Realm
}

I am not sure if they refer to different things, but the fact that they use the same name feels confusing?

I can see that most modules have a "{ModuleName}Result" defined in the "definition" section, so from that point of view, having "ScriptResult" defined in 7.3.1 https://w3c.github.io/webdriver-bidi/#module-script-definition seems consistent. Maybe 7.3.2.6 https://w3c.github.io/webdriver-bidi/#type-script-Result be renamed to avoid reusing the same name?

juliandescottes commented 2 years ago

cc @whimboo, topic we discussed earlier

whimboo commented 2 years ago

@jgraham and @sadym-chromium looks like there is another issue with a non detected broken CDDL type (issue #223 could help here as well).

sadym-chromium commented 2 years ago

WIP

sadym-chromium commented 2 years ago

Fix in #224

@juliandescottes cannot assign you as a reviewer for whatever reason (

juliandescottes commented 2 years ago

Fix in #224

@juliandescottes cannot assign you as a reviewer for whatever reason (

Thanks for the PR! Not sure why I can't be assigned as a reviewer, I probably don't have the appropriate rights on the repo. But I "can" review the PR even if I'm not tagged, there's still that.

(edit: that should actually work now :) )

juliandescottes commented 2 years ago

https://github.com/w3c/webdriver-bidi/pull/224 was merge, we can close this.