EFL's DRT (DumpRenderTreeView) needs to implement navigation_policy_decision in order to unskip fast/encoding/mailto-always-utf-8.html fast/forms/mailto/advanced-get.html fast/forms/mailto/advanced-put.html fast/forms/mailto/formenctype-attribute-button-html.html fast/forms/mailto/formenctype-attribute-input-html.html fast/forms/mailto/get-multiple-items-text-plain.html fast/forms/mailto/get-multiple-items-x-www-form-urlencoded.html fast/forms/mailto/get-multiple-items.html fast/forms/mailto/get-non-ascii-always-utf-8.html fast/forms/mailto/get-non-ascii-text-plain-latin-1.html fast/forms/mailto/get-non-ascii-text-plain.html fast/forms/mailto/get-non-ascii.html fast/forms/mailto/get-overwrite-query.html fast/forms/mailto/post-append-query.html fast/forms/mailto/post-multiple-items-multipart-form-data.html fast/forms/mailto/post-multiple-items-text-plain.html fast/forms/mailto/post-multiple-items-x-www-form-urlencoded.html fast/forms/mailto/post-multiple-items.html fast/forms/mailto/post-text-plain-with-accept-charset.html fast/forms/mailto/post-text-plain.html
Created attachment 139177 [details] navigation_policy_decision + LayoutTestController::setCustomPolicyDelegate
Comment on attachment 139177 [details] navigation_policy_decision + LayoutTestController::setCustomPolicyDelegate Attachment 139177 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12584241
(In reply to comment #2) > (From update of attachment 139177 [details]) > Attachment 139177 [details] did not pass efl-ews (efl): > Output: http://queues.webkit.org/results/12584241 This patch depends on other patch from bug85048.
Comment on attachment 139177 [details] navigation_policy_decision + LayoutTestController::setCustomPolicyDelegate View in context: https://bugs.webkit.org/attachment.cgi?id=139177&action=review > Tools/DumpRenderTree/efl/DumpRenderTreeView.cpp:160 > + String output = String("Policy delegate: attempt to load ") + urlSuitableForTestResult(request->url) + Why don't use use makeString() ?
Created attachment 140175 [details] navigation_policy_decision + LayoutTestController::setCustomPolicyDelegate use makeString
(In reply to comment #4) > (From update of attachment 139177 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139177&action=review > > > Tools/DumpRenderTree/efl/DumpRenderTreeView.cpp:160 > > + String output = String("Policy delegate: attempt to load ") + urlSuitableForTestResult(request->url) + > > Why don't use use makeString() ? Thanks for the hint! Done.
Comment on attachment 140175 [details] navigation_policy_decision + LayoutTestController::setCustomPolicyDelegate Attachment 140175 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12627070
Created attachment 142968 [details] patch v3 (rebased)
Comment on attachment 142968 [details] patch v3 (rebased) View in context: https://bugs.webkit.org/attachment.cgi?id=142968&action=review > Tools/DumpRenderTree/efl/DumpRenderTreeView.cpp:176 > +static Eina_Bool onNavigationPolicyDecision(Ewk_View_Smart_Data*, Ewk_Frame_Resource_Request* request, Ewk_Navigation_Type navigationType ) Style nit : remove a space at the end of this line. > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:213 > + setCustomPolicyDelegate(true, true); Does these test cases assumes policy delegation and permissive are always enabled?
(In reply to comment #9) > > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:213 > > + setCustomPolicyDelegate(true, true); > > Does these test cases assumes policy delegation and permissive are always enabled? LayoutTestController::waitForPolicyDelegate is used in fast/forms/mailto/* (which are unskipped by this patch) and http/tests/download/inherited-encoding.html http/tests/download/inherited-encoding-form-submission-result.html http/tests/download/default-encoding.html For all of the mentioned tests policy delegation enabled flag is crucial as affects logging, permissive flag is not crucial and tests can pass either with or without this flag. However Qt port for instance keeps permissive flag as false so we can do the same keeping consistency.
Created attachment 143008 [details] patch v4 (fixed comments from Gyuyoung)
Comment on attachment 143008 [details] patch v4 (fixed comments from Gyuyoung) Attachment 143008 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12748015
(In reply to comment #12) > (From update of attachment 143008 [details]) > Attachment 143008 [details] did not pass qt-wk2-ews (qt): > Output: http://queues.webkit.org/results/12748015 Sorry, it was false positive alarm.
*** Bug 84842 has been marked as a duplicate of this bug. ***
It looks qt-wk2 ews had some problems. I think you need to submit patch again. Let's see ews result again.
Created attachment 143495 [details] patch v4 (resubmitted)
Created attachment 143497 [details] patch v5(rebased)
Attachment 143497 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 LayoutTests/platform/efl/test_expectations.txt:305: fast/loader/policy-delegate-action-hit-test-zoomed.html is also in a Skipped file. [test/expectations] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 143509 [details] patch v6 (Skipped list clean up)
Comment on attachment 143509 [details] patch v6 (Skipped list clean up) View in context: https://bugs.webkit.org/attachment.cgi?id=143509&action=review > LayoutTests/platform/efl/test_expectations.txt:301 > +// Needs custom policy delegate enhancement to log downloads - EFL port is missing api for getting header fields from HTTP responses. It would be nice to open bug reports for those. We try to avoid using BUGWKEFL in test_expectations.txt > LayoutTests/platform/efl/test_expectations.txt:304 > +// Needs custom policy delegate enhancement to log DOM node info - EFL port is missing DOM node abstraction Ditto. > Tools/DumpRenderTree/efl/DumpRenderTreeView.cpp:181 > + const String& output = makeString("Policy delegate: attempt to load ", urlSuitableForTestResult(request->url), Why construct a WTF::String here? You could just put all that as printf arguments.
Created attachment 145091 [details] patch v7 (Review comments are met. Thanks Chris)
Comment on attachment 145091 [details] patch v7 (Review comments are met. Thanks Chris) LGTM. Thanks.
Comment on attachment 145091 [details] patch v7 (Review comments are met. Thanks Chris) View in context: https://bugs.webkit.org/attachment.cgi?id=145091&action=review Looks good to me. > LayoutTests/platform/efl/test_expectations.txt:356 > +// Needs custom policy delegate enhancement to log downloads - EFL port is missing api for getting header fields from HTTP responses. Does *api* needs uppercase ?
Comment on attachment 145091 [details] patch v7 (Review comments are met. Thanks Chris) View in context: https://bugs.webkit.org/attachment.cgi?id=145091&action=review >> LayoutTests/platform/efl/test_expectations.txt:356 >> +// Needs custom policy delegate enhancement to log downloads - EFL port is missing api for getting header fields from HTTP responses. > > Does *api* needs uppercase ? Right.
Comment on attachment 145091 [details] patch v7 (Review comments are met. Thanks Chris) Clearing flags on attachment: 145091 Committed r119116: <http://trac.webkit.org/changeset/119116>
All reviewed patches have been landed. Closing bug.