RESOLVED FIXED 87374
Reorder arguments to compare() in the QML WebView tests
https://bugs.webkit.org/show_bug.cgi?id=87374
Summary Reorder arguments to compare() in the QML WebView tests
Alexander Færøy
Reported 2012-05-24 05:02:37 PDT
SSIA.
Attachments
Patch (2.24 KB, patch)
2012-05-24 05:07 PDT, Alexander Færøy
no flags
Patch (2.60 KB, patch)
2012-05-24 05:25 PDT, Alexander Færøy
hausmann: review+
Alexander Færøy
Comment 1 2012-05-24 05:07:07 PDT
Caio Marcelo de Oliveira Filho
Comment 2 2012-05-24 05:12:49 PDT
Comment on attachment 143789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143789&action=review > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml:-56 > - // Delayed windowShown to workaround problems with Qt5 in debug mode. > - when: false > - Timer { > - running: parent.windowShown > - repeat: false > - interval: 1 > - onTriggered: parent.when = true > - } ChangeLog doesn't talk about this, that is the most significant change. If you don't mention feels like it was an accident. Do other files still have this workaround? If so, can it be removed from them too?
Simon Hausmann
Comment 3 2012-05-24 05:13:48 PDT
Comment on attachment 143789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143789&action=review > Source/WebKit2/ChangeLog:7 > + Reorder arguments to compare() in the QML WebView tests > + https://bugs.webkit.org/show_bug.cgi?id=87374 > + > + Reviewed by NOBODY (OOPS!). > + I'm missing a _why_ here, i.e. why do you re-order the arguments? I can make an guess myself, but it would be nicer without me and potentially others guessing :) > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml:-57 > - // Delayed windowShown to workaround problems with Qt5 in debug mode. > - when: false > - Timer { > - running: parent.windowShown > - repeat: false > - interval: 1 > - onTriggered: parent.when = true > - } > - I suggest to mention in the ChangeLog that you removed this workaround because it's not necessary anymore. (Is that correct?)
Alexander Færøy
Comment 4 2012-05-24 05:25:45 PDT
Alexander Færøy
Comment 5 2012-05-24 05:51:43 PDT
Note You need to log in before you can comment on or make changes to this bug.