[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


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #105837|review?                     |review-
               Flag|                            |




--- Comment #2 from Benjamin Poulain <benjamin at webkit.org>  2011-08-31 15:17:40 PST ---
(From update of attachment 105837)
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();
}

> Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:248
> +        QString u(url);

not a fan of "u" for the variable name.

> 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.

> Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:708
> +    QByteArray urlData = pathToLocalResource(url).toAscii();

.toAscii() is evil, you should use toUtf8() or toLatin1()

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