Bug 84425 - [EFL] EFL's LayoutTestController does not implement elementDoesAutoCompleteForElementWithId
Summary: [EFL] EFL's LayoutTestController does not implement elementDoesAutoCompleteFo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-19 22:48 PDT by Chris Dumez
Modified: 2012-05-06 22:51 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.01 KB, patch)
2012-04-19 22:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.92 KB, patch)
2012-04-22 23:19 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.84 KB, patch)
2012-04-23 06:40 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-04-19 22:48:47 PDT
EFL's LayoutTestController does not implement elementDoesAutoCompleteForElementWithId. Implementing this would allow the following test case to be unskipped:
  * security/set-form-autocomplete-attribute.html
Comment 1 Chris Dumez 2012-04-19 22:59:57 PDT
Created attachment 138044 [details]
Patch
Comment 2 Gyuyoung Kim 2012-04-20 15:45:01 PDT
Comment on attachment 138044 [details]
Patch

It looks this patch refers to QT port. Looks fine to me,.
Comment 3 Raphael Kubo da Costa (:rakuco) 2012-04-22 19:47:50 PDT
Comment on attachment 138044 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138044&action=review

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:114
> +    if (!coreNode || !coreNode->renderer())

Are you sure you need the second check here? None of the methods below seem to use it.

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:117
> +    WebCore::HTMLInputElement* inputElement = static_cast< WebCore::HTMLInputElement*>(coreNode);

Style nit: extra space after the opening '<'.

> Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:472
> +    return DumpRenderTreeSupportEfl::elementDoesAutoCompleteForElementWithId(mainFrame, id->ustring().utf8().data());

I think it makes sense to construct a String here and pass it as the second parameter to the DRTSupportEfl method:
  const String elementId(id->ustring().impl());
Comment 4 Chris Dumez 2012-04-22 23:19:11 PDT
Created attachment 138295 [details]
Patch

Update patch to take feedback into consideration.
Comment 5 Raphael Kubo da Costa (:rakuco) 2012-04-23 06:27:16 PDT
Comment on attachment 138295 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138295&action=review

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:119
> +    if (!inputElement)
> +        return false;

Is this condition reachable? The previous if already checks if coreNode is 0.
Comment 6 Chris Dumez 2012-04-23 06:40:30 PDT
Created attachment 138336 [details]
Patch

Yes, you're right. I copied that from the Qt port without paying enough attention.
Comment 7 Raphael Kubo da Costa (:rakuco) 2012-04-23 07:25:37 PDT
Comment on attachment 138336 [details]
Patch

Looks good, thank you.
Comment 8 WebKit Review Bot 2012-04-23 09:10:54 PDT
Comment on attachment 138336 [details]
Patch

Clearing flags on attachment: 138336

Committed r114906: <http://trac.webkit.org/changeset/114906>
Comment 9 WebKit Review Bot 2012-04-23 09:10:59 PDT
All reviewed patches have been landed.  Closing bug.