Bug 32555 – [Qt] support navigator.onLine and ononline/onoffline events.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Yael]

A patch is under testing and should be submitted soon.

[Yael]

Implementation of navigator.onLine and ononline/onoffline is trivial, and could be submitted as such. The problem is that I need the ability to "turn the network off" from the application UI.
When this happens, navigator.onLine should return false, and onoffline event should be fired. When the application UI allows network access, ononline event should be fired, if the device is online at that time.

[Yael]

Created attachment 45016 [details]
Patch.

This is a temporary solution, S60 only, until Bearer Management becomes part of QtNetwor (in Qt 4.7). At that time, the implementation should be reviesed.
check-webkit-style generates an error on including the new moc file, and I am not sure how to fix that error. seems to me that moc files should be excluded from that script.

[WebKit Review Bot]

Attachment 45016 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/network/qt/NetworkStateNotifierQt.cpp:90:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1

[Laszlo Gombos]

Comment on attachment 45016 [details]
Patch.

Yael, thanks for taking on this, the core of the patch looks really good.

I think it would be a small extra effort to support not only Symbian but all the platforms that Bearer Management library supports (most notably Maemo).

> +        NetworkAccessAllowed

I'm not sure I understand the need for the new setting in QtWebKit. I would think that onlineStateChanged fires if the network is turned off from the "application UI". 

> +symbian: {
> +    exists($${EPOCROOT}epoc32/release/winscw/udeb/QtBearer_tp.lib)| \
> +    exists($${EPOCROOT}epoc32/release/armv5/lib/QtBearer_tp.lib) {
> +        DEFINES += ENABLE_QT_BEARER=1
> +        LIBS += -lQtBearer_tp
> +    }
> +}

I think the library name should be QtBearer instead of QtBearer_tp (I think tp stands for Technology Preview here).

AFAIK the convention for this type of build flag would be QT_NO_BEARER_MANAGEMENT or QT_NO_BEARER. Also if bearer management library only works with Qt 4.6 (or later) we should also check for Qt version (in the source files not in the build file).

> +contains(DEFINES, ENABLE_QT_BEARER=1) {
> +    SOURCES += \
> +        platform/network/qt/NetworkStateNotifierQt.cpp
> +
> +    HEADERS += \
> +        platform/network/qt/NetworkStateNotifierPrivate.h
> +}

I prefer to always add the source and header files to the list of files to build and add the guard to the header/cpp file.

Setting it to r- as I think it needs a bit more work, but as I said fundamentally the change looks good.

[Yael]

Created attachment 45079 [details]
Patch v2

Address Laszlo's comments and also split the patch to HTML5 feature only, the UI API will be a separate patch.

[WebKit Review Bot]

Attachment 45079 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/network/qt/NetworkStateNotifierQt.cpp:73:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1

[Yael]

(In reply to comment #6)
> Attachment 45079 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/platform/network/qt/NetworkStateNotifierQt.cpp:73:  Alphabetical
> sorting problem.  [build/include_order] [4]
> Total errors found: 1

Filed https://bugs.webkit.org/show_bug.cgi?id=32669 about wrong error from check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 45079 [details]
Patch v2

Clearing flags on attachment: 45079

Committed r52261: <http://trac.webkit.org/changeset/52261>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

> Filed https://bugs.webkit.org/show_bug.cgi?id=32669 about wrong error from
> check-webkit-style.

Thank you for filing the bug.  :)

[Simon Hausmann]

Backported as commit eb1c86da9f5fa43a0e126ef58ab11cbd4d200af7 in qtwebkit-4.6 in http://gitorious.org/+qtwebkit-developers/webkit/qtwebkit