[Webkit-unassigned] [Bug 67254] [Qt][DRT] Normalize file:///tmp/LayoutTests in LayoutTestController::pathToLocalResource()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67254





--- Comment #3 from Jarred Nicholls <jarred at sencha.com>  2011-08-31 17:10:18 PST ---
(In reply to comment #2)
> (From update of attachment 105837 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105837&action=review
> 
> Suggestion:
> Create a temp variable for "file:///tmp/LayoutTests/", and use it afterwards. Pseudo-ish code:
> const Qstring localTmpUrl(QLatin1String("file:///tmp/LayoutTests/"));
> if (url.startsWith(localTmpUrl) {
>     QFileInfo layoutTestsRoot(QCoreApplication::applicationDirPath() + "/../../../LayoutTests/");
>     if (layoutTestsRoot.exists())
>         return url.right(localTmpUrl.length()) + layoutTestsRoot.absolutePath();
> }

I'll do this and fix any tests that have four slashes e.g. file:////tmp/LayoutTests (that's the reason for indexOf)

> 
> > Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:248
> > +        QString u(url);
> 
> not a fan of "u" for the variable name.

Me neither, but I'll avoid a non-const string.

> 
> > Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:252
> > +        // https://bugs.webkit.org/show_bug.cgi?id=67254
> 
> I would not add the bug number in this comment, it implies this bug report is about fixing the test on Windows due to the symlink issue.

Good call

> 
> > Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:708
> > +    QByteArray urlData = pathToLocalResource(url).toAscii();
> 
> .toAscii() is evil, you should use toUtf8() or toLatin1()

Agreed, wanted to keep consistent with what's there but definitely agree.  Shouldn't bother data: encoded URLs.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.