Bug 87374 – Reorder arguments to compare() in the QML WebView tests

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Alexander Færøy]

SSIA.

[Alexander Færøy]

Created attachment 143789 [details]
Patch

[Caio Marcelo de Oliveira Filho]

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

Created attachment 143794 [details]
Patch

[Alexander Færøy]

Committed r118355: <http://trac.webkit.org/changeset/118355>