Bug 87374

Summary: Reorder arguments to compare() in the QML WebView tests
Product: WebKit Reporter: Alexander Færøy <ahf>
Component: WebKit QtAssignee: Alexander Færøy <ahf>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, kenneth, menard, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch hausmann: review+

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>