Bug 32555 - [Qt] support navigator.onLine and ononline/onoffline events.
Summary: [Qt] support navigator.onLine and ononline/onoffline events.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Yael
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-12-15 05:43 PST by Yael
Modified: 2009-12-29 04:44 PST (History)
5 users (show)

See Also:


Attachments
Patch. (11.94 KB, patch)
2009-12-16 14:52 PST, Yael
laszlo.gombos: review-
Details | Formatted Diff | Diff
Patch v2 (7.26 KB, patch)
2009-12-17 09:47 PST, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2009-12-15 05:43:11 PST
A patch is under testing and should be submitted soon.
Comment 1 Yael 2009-12-15 13:29:51 PST
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.
Comment 2 Yael 2009-12-16 14:52:56 PST
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.
Comment 3 WebKit Review Bot 2009-12-16 14:57:22 PST
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
Comment 4 Laszlo Gombos 2009-12-16 16:57:30 PST
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.
Comment 5 Yael 2009-12-17 09:47:11 PST
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.
Comment 6 WebKit Review Bot 2009-12-17 09:48:11 PST
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
Comment 7 Yael 2009-12-17 10:02:47 PST
(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.
Comment 8 WebKit Commit Bot 2009-12-17 10:39:50 PST
Comment on attachment 45079 [details]
Patch v2

Clearing flags on attachment: 45079

Committed r52261: <http://trac.webkit.org/changeset/52261>
Comment 9 WebKit Commit Bot 2009-12-17 10:39:55 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Adam Barth 2009-12-17 15:18:25 PST
> Filed https://bugs.webkit.org/show_bug.cgi?id=32669 about wrong error from
> check-webkit-style.

Thank you for filing the bug.  :)
Comment 11 Simon Hausmann 2009-12-29 04:44:27 PST
Backported as commit eb1c86da9f5fa43a0e126ef58ab11cbd4d200af7 in qtwebkit-4.6 in http://gitorious.org/+qtwebkit-developers/webkit/qtwebkit