[webkit-reviews] review granted: [Bug 58665] [WK2] Should strip file:// urls before adding to console message : [Attachment 89828] fix patch 3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 15 12:40:25 PDT 2011


Alexey Proskuryakov <ap at webkit.org> has granted Chang Shu <cshu at webkit.org>'s
request for review:
Bug 58665: [WK2] Should strip file:// urls before adding to console message
https://bugs.webkit.org/show_bug.cgi?id=58665

Attachment 89828: fix patch 3
https://bugs.webkit.org/attachment.cgi?id=89828&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89828&action=review

r=me unless you decide to switch to KURL, which would require a new review.

> Tools/ChangeLog:10
> +	   (WTR::lastPathComponent):

This function has a different name now.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:751
> +static string lastURLPathComponent(const string& path)

I just realized that since we're asserting that it starts with file://, the
name should be lastFileURLPathComponent(). Sorry for a bad suggestion
previously.

Also, can we use KURL here? That would obviously be much cleaner than writing
custom code.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:757
> +    if (!tmpPath.empty()) {

We prefer early returns to indenting code in if blocks.


More information about the webkit-reviews mailing list