Bug 87374 - Reorder arguments to compare() in the QML WebView tests
Summary: Reorder arguments to compare() in the QML WebView tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexander Færøy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-24 05:02 PDT by Alexander Færøy
Modified: 2012-05-24 05:51 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.24 KB, patch)
2012-05-24 05:07 PDT, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (2.60 KB, patch)
2012-05-24 05:25 PDT, Alexander Færøy
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Færøy 2012-05-24 05:02:37 PDT
SSIA.
Comment 1 Alexander Færøy 2012-05-24 05:07:07 PDT
Created attachment 143789 [details]
Patch
Comment 2 Caio Marcelo de Oliveira Filho 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?
Comment 3 Simon Hausmann 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?)
Comment 4 Alexander Færøy 2012-05-24 05:25:45 PDT
Created attachment 143794 [details]
Patch
Comment 5 Alexander Færøy 2012-05-24 05:51:43 PDT
Committed r118355: <http://trac.webkit.org/changeset/118355>