Summary: | WebDriver: implement element property command | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebDriver | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bburg, ews-watchlist, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2017-12-01 06:02:18 PST
Created attachment 328089 [details]
Patch
Attachment 328089 [details] did not pass style-queue:
ERROR: Source/WebDriver/WebDriverService.h:87: Extra space before ( in function call [whitespace/parens] [4]
ERROR: Source/WebDriver/WebDriverService.h:88: Extra space before ( in function call [whitespace/parens] [4]
ERROR: Source/WebDriver/Session.h:89: Extra space before ( in function call [whitespace/parens] [4]
ERROR: Source/WebDriver/Session.h:90: Extra space before ( in function call [whitespace/parens] [4]
ERROR: Source/WebDriver/WebDriverService.cpp:1137: Extra space before ( in function call [whitespace/parens] [4]
ERROR: Source/WebDriver/Session.cpp:1287: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 6 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 328089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328089&action=review r=me > Source/WebDriver/Session.cpp:1321 > + return; It's not clear to me what will happen if element.`property` is undefined. Will it return an error or not? Just looking at the W3C test names, I can't easily tell that this case is covered. (In reply to Brian Burg from comment #3) > Comment on attachment 328089 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328089&action=review > > r=me > > > Source/WebDriver/Session.cpp:1321 > > + return; > > It's not clear to me what will happen if element.`property` is undefined. > Will it return an error or not? Just looking at the W3C test names, I can't > easily tell that this case is covered. Good point, I think it will return a JS error, because the code will raise an exception, I guess. We should handle that case and return null. (In reply to Carlos Garcia Campos from comment #4) > (In reply to Brian Burg from comment #3) > > Comment on attachment 328089 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=328089&action=review > > > > r=me > > > > > Source/WebDriver/Session.cpp:1321 > > > + return; > > > > It's not clear to me what will happen if element.`property` is undefined. > > Will it return an error or not? Just looking at the W3C test names, I can't > > easily tell that this case is covered. > > Good point, I think it will return a JS error, because the code will raise > an exception, I guess. We should handle that case and return null. I've checked this and it's tested by test_element_non_existent. We need to handle it, but in a more general way, so I'll fix this and other similar cases in a follow up patch. Committed r225474: <https://trac.webkit.org/changeset/225474> |