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+

Carlos Garcia Campos
Reported 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.
Attachments
Patch (9.04 KB, patch)
2020-09-14 05:33 PDT, Carlos Garcia Campos 		hi: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2020-09-14 05:33:03 PDT
Created attachment 408702 [details] Patch
Devin Rousso
Comment 2 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 😞
Carlos Garcia Campos
Comment 3 2020-09-14 23:25:12 PDT
Committed r267078: <https://trac.webkit.org/changeset/267078>
Radar WebKit Bug Importer
Comment 4 2020-09-14 23:26:18 PDT
<rdar://problem/68903288>
Carlos Garcia Campos
Comment 5 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.
Note You need to log in before you can comment on or make changes to this bug.