It seems -graphicssystem command line argument isn't supported by Qt5: // ### Qt4 compatibility, remove? static inline void setGraphicsSystem(const QString &) {} But ORWT still pass this argument (-graphicssystem raster) to QtTestBrowser, and QtTestBrowser open an empty window with URL-bar: http://raster and "Host not found" text. We shouldn't pass "-graphicssystem raster" argument if Qt version >= 5.
Created attachment 116503 [details] proposed fix
Comment on attachment 116503 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=116503&action=review I think the idea is good. The ChangeLog entry is missing and I have a few nitpicks :) > Tools/Scripts/old-run-webkit-tests:242 > + } elsif (getQtVersion() eq "5.0") { > + $platform = "qt-5.0"; This seems unrelated? > Tools/Scripts/old-run-webkit-tests:1222 > + if(!(getQtVersion() ge "5.0")) { Shouldn't that properly check the major version of Qt only? (if major-version < 4)
Comment on attachment 116503 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=116503&action=review >> Tools/Scripts/old-run-webkit-tests:242 >> + $platform = "qt-5.0"; > > This seems unrelated? No, we need this change too, because qt-5.0 platform wasn't supported by ORWT before. >> Tools/Scripts/old-run-webkit-tests:1222 >> + if(!(getQtVersion() ge "5.0")) { > > Shouldn't that properly check the major version of Qt only? (if major-version < 4) Of course we can check only the major version, but getQtVersion() (webkitdirs.pm) parses qmake's output and return major&minor version in this format as string. I think, that if ((getQtVersion() lt "5.0")) would be good, because it is equivalent to (non-existent-yet) (getQtMajorVersion() < 5).
Created attachment 116592 [details] proposed fix
Comment on attachment 116592 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=116592&action=review > Tools/ChangeLog:1 > +2011-11-25 Ãdám Kallai <Kallai.Adam@stud.u-szeged.hu> It looks like your first name isn't properly utf-8 encoded here. Is that possible? > Tools/Scripts/old-run-webkit-tests:242 > + } elsif (getQtVersion() eq "5.0") { > + $platform = "qt-5.0"; This looks like an unrelated change. Is that intentional? > Tools/Scripts/old-run-webkit-tests:1224 > - unshift @configurationArgs, qw(-graphicssystem raster -style windows); > + unshift @configurationArgs, qw(-style windows); > + if(getQtVersion() lt "5.0") { > + unshift @configurationArgs, qw(-graphicssystem raster); > + } This looks much better :). Out of curiosity: Does 'lt' work with strings like "5.0" like that?
Ooops, I didn't see Ossy's comment. Let me see again :)
(In reply to comment #3) > (From update of attachment 116503 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116503&action=review > > >> Tools/Scripts/old-run-webkit-tests:242 > >> + $platform = "qt-5.0"; > > > > This seems unrelated? > > No, we need this change too, because qt-5.0 platform wasn't supported by ORWT before. Shouldn't this be mentioned in the ChangeLog then? Right now the title of the bug is about not passing -graphicssystem raster when running OWRT with Qt 5. I do however have the impression that the title of this bug should be "Add support for using OWRT with Qt 5" or similar, and that the ChangeLog should mention this line.
Comment on attachment 116592 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=116592&action=review >> Tools/ChangeLog:1 >> +2011-11-25 Ãdám Kallai <Kallai.Adam@stud.u-szeged.hu> > > It looks like your first name isn't properly utf-8 encoded here. Is that possible? I think pretty diff can't handle correct the utf8 names. Check one of my patches: https://bugs.webkit.org/attachment.cgi?id=114235&action=review or one of Tor Arne's patches: https://bugs.webkit.org/attachment.cgi?id=104833&action=review >> Tools/Scripts/old-run-webkit-tests:1224 >> + } > > This looks much better :). Out of curiosity: Does 'lt' work with strings like "5.0" like that? Unfortunately perl differs from bash. lt, eq, gt, ... are for string comparision in perl.
(In reply to comment #7) > (In reply to comment #3) > > (From update of attachment 116503 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=116503&action=review > > > > >> Tools/Scripts/old-run-webkit-tests:242 > > >> + $platform = "qt-5.0"; > > > > > > This seems unrelated? > > > > No, we need this change too, because qt-5.0 platform wasn't supported by ORWT before. > > Shouldn't this be mentioned in the ChangeLog then? You're right, we should. > Right now the title of the bug is about not passing -graphicssystem raster when running OWRT with Qt 5. I do however have the impression that the title of this bug should be "Add support for using OWRT with Qt 5" or similar, and that the ChangeLog should mention this line. It's reasonable, I fixed the title of the bug.
Created attachment 116608 [details] proposed fix
Comment on attachment 116608 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=116608&action=review LGTM, r=me. > Tools/Scripts/old-run-webkit-tests:1222 > + if(getQtVersion() lt "5.0") { only a little nit: missing space after the if.(I'll fix it locally before landing.)
Landed in http://trac.webkit.org/changeset/101169