Bug 65315 – [Qt] Make QDesktopWebView/QTouchWebView loadProgress property more usable in QML.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alexis Menard (darktears)]

[Qt] Make QDesktopWebView loadProgress property more usable in QML.

[Alexis Menard (darktears)]

Created attachment 102254 [details]
Patch

[Noam Rosenthal]

Comment on attachment 102254 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102254&action=review

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:54
> +    Q_PROPERTY(int loadProgress READ loadProgress NOTIFY loadProgress)

Shouldn't NOTIFY point to a signal?

[Benjamin Poulain]

Comment on attachment 102254 [details]
Patch

I am pretty sure the other view could enjoy the same kind of love :)
Please update both view together when they provide the same API. Storing the loadProgress should probably be done on QtWebPageProxy.

[Caio Marcelo de Oliveira Filho]

Comment on attachment 102254 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102254&action=review

>> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:54
>> +    Q_PROPERTY(int loadProgress READ loadProgress NOTIFY loadProgress)
> 
> Shouldn't NOTIFY point to a signal?

Yes, and it does :-)... The problem here is that the signal and the getter have the same name. Alexis, what about changing the signal name to loadProgressChanged?

[Alexis Menard (darktears)]

Created attachment 102259 [details]
Patch

[Benjamin Poulain]

Comment on attachment 102259 [details]
Patch

QTouchWebPage already has the property. qtouchwebview_p.h is not the implementation of ViewInterface like on the desktop case. The update should be on QTouchWebPage :)
Would you mind addind an API test on the way? :-D

[Alexis Menard (darktears)]

Created attachment 102267 [details]
Patch

[Benjamin Poulain]

Comment on attachment 102267 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102267&action=review

> Source/WebKit2/UIProcess/API/qt/tests/commonviewtests/tst_commonviewtests.cpp:160
> +    QVERIFY(waitForSignal(viewAbstraction.data(), SIGNAL(loadProgressChanged(int))));
> +

I think this should be a signal spy over loadProgressChanged(), then you check that you got the signal. The wait for signal should be on the loadSucceeded only, like the next line.
This is just for robustness, in case a future refactoring make it simultaneous to have loadProgressChanged(100) and loadSucceeded() for local file.

[Alexis Menard (darktears)]

Committed r91985: <http://trac.webkit.org/changeset/91985>