Bug 30268 – [Qt] Layouttest fast/css/opacity-float.html failed on Qt

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chang Shu]

run
WebKitBuild/Release/bin/DumpRenderTree --qt LayoutTests/fast/css/opacity-float.html

error message:
CONSOLE MESSAGE: line 6: TypeError: Result of expression
'layoutTestController.setPOSIXLocale' [undefined] is not a function.

[Chang Shu]

Created attachment 41041 [details]
implement setPOSIXLocale

[Simon Hausmann]

Comment on attachment 41041 [details]
implement setPOSIXLocale

I think your patch looks good, but I think qt/DumpRenderTree.cpp's resetToConsistentStateBeforeTesting() should also set the default locale back (i.e. using QLocale::setDefault).

Could you add that line, just so that we are safe that after a test called setPOSIXLocale we set this back and avoid side-effects in other tests?

[Chang Shu]

(In reply to comment #2)
> (From update of attachment 41041 [details])
> I think your patch looks good, but I think qt/DumpRenderTree.cpp's
> resetToConsistentStateBeforeTesting() should also set the default locale back
> (i.e. using QLocale::setDefault).
> 
> Could you add that line, just so that we are safe that after a test called
> setPOSIXLocale we set this back and avoid side-effects in other tests?

Thanks for the comments! I was asking the same question to Evan in bug 29653 and now I forgot to do it myself. :(

Btw, I see there is already a line of "setlocale(LC_ALL,"");" in function resetToConsistentStateBeforeTesting(). However, I don't think it works for Qt. Shall I remove it and replace with the following line?
    QLocale qlocale;
    QLocale::setDefault(qlocale);

Thanks!

[Evan Martin]

I added that setlocale line in a vain attempt to not break the Qt build (I thought you were sharing LayoutTestController to).  It is safe to remove if you are resetting the locale.

[Chang Shu]

Created attachment 41103 [details]
reset locale at the beginning of each test case

[Chang Shu]

(In reply to comment #4)
> I added that setlocale line in a vain attempt to not break the Qt build (I
> thought you were sharing LayoutTestController to).  It is safe to remove if you
> are resetting the locale.

Thanks, Evan. New patch attached.

[Simon Hausmann]

Comment on attachment 41103 [details]
reset locale at the beginning of each test case

Thanks! Your patch looks okay, but it is missing a ChangeLog entry. Can you add that? :)

[Chang Shu]

Created attachment 41111 [details]
add missing ChangeLog

forgot to include ChangeLog last time

[Simon Hausmann]

Comment on attachment 41111 [details]
add missing ChangeLog

After the patch from bug 30305 this may require a rebase, but otherwise it looks good.

[Chang Shu]

Thanks for the review!

[Chang Shu]

Yeah, a re-work is a must. Will submit a new patch.

[Chang Shu]

Created attachment 41148 [details]
re-work after baseline change

[Simon Hausmann]

Comment on attachment 41148 [details]
re-work after baseline change

Thanks!

[WebKit Commit Bot]

Comment on attachment 41148 [details]
re-work after baseline change

Clearing flags on attachment: 41148

Committed r49563: <http://trac.webkit.org/changeset/49563>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.