Bug 216477

Summary: REGRESSION(r266885): WebDriver is broken since r266885
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebDriverAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, hi, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=216302
Attachments:
Description Flags
Patch hi: review+

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.