Bug 119585 - [EFL][WK1] URL printing code in DumpRenderTree doesn't match WTR or Mac DRT
Summary: [EFL][WK1] URL printing code in DumpRenderTree doesn't match WTR or Mac DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-08 11:30 PDT by Alexey Proskuryakov
Modified: 2013-08-12 22:54 PDT (History)
6 users (show)

See Also:

Attachments
Patch (8.42 KB, patch)
2013-08-12 19:10 PDT, Ryuan Choi 		no flags Details | Formatted Diff | Diff
StyleFixed (8.43 KB, patch)
2013-08-12 22:05 PDT, Ryuan Choi 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2013-08-08 11:30:40 PDT 
pathSuitableForTestResult() in DumpRenderTreeChrome has some code that doesn't match other platforms.

If should return a path that's relative to main frame URL, or just the file name if the resource is not in the same directory subtree.
Comment 1 Chris Dumez 2013-08-12 03:00:20 PDT 
Adding Ryuan and Gyuyoung in CC since I believe they are maintaining EFL WK1.
Comment 2 Gyuyoung Kim 2013-08-12 05:50:03 PDT 
ap, will you submit a patch for this bug ? Otherwise, do you want we make a patch for this bug ?
Comment 3 Chris Dumez 2013-08-12 05:59:17 PDT 
(In reply to comment #2)
> ap, will you submit a patch for this bug ? Otherwise, do you want we make a patch for this bug ?

I really think it is up to us to fix EFL bugs.
Comment 4 Gyuyoung Kim 2013-08-12 06:02:26 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > ap, will you submit a patch for this bug ? Otherwise, do you want we make a patch for this bug ?
> 
> I really think it is up to us to fix EFL bugs.

Looks like that as well. Ryuan or I will fix this soon. ;)
Comment 5 Alexey Proskuryakov 2013-08-12 08:47:25 PDT 
Thank you! It would be much better for someone with a EFL build to do this, as there is likely to be quite a bit of test expectation cleanup.

WebKitTestRunner code can probably serve as a model here.
Comment 6 Ryuan Choi 2013-08-12 19:10:31 PDT 
Created attachment 208583 [details]
Patch
Comment 7 Ryuan Choi 2013-08-12 19:20:56 PDT 
(In reply to comment #5)
> Thank you! It would be much better for someone with a EFL build to do this, as there is likely to be quite a bit of test expectation cleanup.
> 
> WebKitTestRunner code can probably serve as a model here.

WebKitTestRunner was good reference.
I made patch similarly.
Comment 8 Gyuyoung Kim 2013-08-12 19:26:05 PDT 
Comment on attachment 208583 [details]
Patch

This patch lets the EFL port follow r153852. LGTM. However, ap might want to have final look before landing.
Comment 9 Alexey Proskuryakov 2013-08-12 20:25:25 PDT 
Comment on attachment 208583 [details]
Patch

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

Looks good to me too.

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:226
> +const WebCore::KURL DumpRenderTreeSupportEfl::provisionalUrl(const Evas_Object* ewkFrame)

WebKit style would be "provisionalURL". I do not know if this code is intended to follow common WebKit style.

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:350
> +    KURL mainFrameUrl = KURL(ParsedURLString, ewk_frame_uri_get(browser->mainFrame()));

Ditto.
Comment 10 Ryuan Choi 2013-08-12 22:05:25 PDT 
Created attachment 208593 [details]
StyleFixed
Comment 11 WebKit Commit Bot 2013-08-12 22:54:49 PDT 
Comment on attachment 208593 [details]
StyleFixed

Clearing flags on attachment: 208593

Committed r153977: <http://trac.webkit.org/changeset/153977>
Comment 12 WebKit Commit Bot 2013-08-12 22:54:52 PDT 
All reviewed patches have been landed.  Closing bug.