Bug 216477 - REGRESSION(r266885): WebDriver is broken since r266885
Summary: REGRESSION(r266885): WebDriver is broken since r266885
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebDriver (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-14 05:23 PDT by Carlos Garcia Campos
Modified: 2020-09-14 23:26 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.04 KB, patch)
2020-09-14 05:33 PDT, Carlos Garcia Campos
drousso: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2020-09-14 05:23:33 PDT
There are several issues introduced in r266885, most of them related to JSValue::get methods now returning an optional. In some cases of boolean values, the optional is checked instead of the contained bool or the logic is inverted. JSONValue::getString() is problematic too because it doesn't use optional, when you call getString() and null is returned it could be because the property is not present in the object or because the property is not a string. In several cases we need to know that so we need to first get the value and then call asString(9 instead of calling getString() directly.
Comment 1 Carlos Garcia Campos 2020-09-14 05:33:03 PDT
Created attachment 408702 [details]
Patch
Comment 2 Devin Rousso 2020-09-14 11:18:47 PDT
Comment on attachment 408702 [details]
Patch

r=me, wow I can't believe I missed some of these subtleties 😳 thanks for fixing!

Is there no EWS for WebDriver or anything?  I wouldn't have landed the change if I'd known that things were broken 😞
Comment 3 Carlos Garcia Campos 2020-09-14 23:25:12 PDT
Committed r267078: <https://trac.webkit.org/changeset/267078>
Comment 4 Radar WebKit Bug Importer 2020-09-14 23:26:18 PDT
<rdar://problem/68903288>
Comment 5 Carlos Garcia Campos 2020-09-14 23:26:41 PDT
(In reply to Devin Rousso from comment #2)
> Comment on attachment 408702 [details]
> Patch
> 
> r=me, wow I can't believe I missed some of these subtleties 😳 thanks for
> fixing!
> 
> Is there no EWS for WebDriver or anything?  I wouldn't have landed the
> change if I'd known that things were broken 😞

No, we don't have EWS for WebDriver, the bots detected the issue.