Bug 180244 - WebDriver: implement element property command
Summary: WebDriver: implement element property command
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: 2017-12-01 06:02 PST by Carlos Garcia Campos
Modified: 2017-12-04 03:30 PST (History)
3 users (show)

See Also:


Attachments
Patch (9.08 KB, patch)
2017-12-01 06:05 PST, Carlos Garcia Campos
bburg: 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 2017-12-01 06:02:18 PST
13.3 Get Element Property
https://w3c.github.io/webdriver/webdriver-spec.html#get-element-property
Comment 1 Carlos Garcia Campos 2017-12-01 06:05:44 PST
Created attachment 328089 [details]
Patch
Comment 2 Build Bot 2017-12-01 06:08:24 PST
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 3 Brian Burg 2017-12-01 09:29:52 PST
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.
Comment 4 Carlos Garcia Campos 2017-12-02 07:07:17 PST
(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.
Comment 5 Carlos Garcia Campos 2017-12-04 03:27:53 PST
(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.
Comment 6 Carlos Garcia Campos 2017-12-04 03:29:37 PST
Committed r225474: <https://trac.webkit.org/changeset/225474>
Comment 7 Radar WebKit Bug Importer 2017-12-04 03:30:32 PST
<rdar://problem/35826863>