Bug 35795 – [Qt] Initial Orbit support in Qt WebKit

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Qt

[Yael]

Orbit is a new UI style in for Qt. <http://developer.symbian.org/wiki/index.php/Orbit>.
Some changes are needed in WebKit to support it.

[Yael]

Created attachment 50105 [details]
Patch v1

This is the first in a series of patches for adding Orbit support to WebKit.
This patch addresses only some type conversions and Orbit dialogs and does not address HbStyle yet.

[WebKit Review Bot]

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

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/HbLauncher/utils.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/urlloader.cpp:29:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/main.cpp:33:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.cpp"
WebKit/qt/HbLauncher/mainwindow.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/locationedit.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/webview.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.h"
WebKit/qt/HbLauncher/webpage.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 7 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 50105 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/336485

[Yael]

Created attachment 50107 [details]
Fix build in QWebPage.cpp.

Not sure what to do with the style errors. The affected files cannot include config.h, as the errors suggest.

[WebKit Review Bot]

Attachment 50107 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/336499

[WebKit Review Bot]

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

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/HbLauncher/utils.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/urlloader.cpp:29:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/main.cpp:33:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.cpp"
WebKit/qt/HbLauncher/mainwindow.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/locationedit.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/webview.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.h"
WebKit/qt/HbLauncher/webpage.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 7 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

check-webkit-style may need some tweaking.  Please file bugs.  Also this does not build on Qt.  Is that because the Qt bot is still 4.5 and this requires 4.6?

[Yael]

(In reply to comment #7)
> check-webkit-style may need some tweaking.  Please file bugs.  Also this does
> not build on Qt.  Is that because the Qt bot is still 4.5 and this requires
> 4.6?

I am not sure how to fix check-webkit-style for this specific problem. The affected files are the test application, and as such it should link only with public API, and should not include config,h.
The build error was real. Since my laptop takes forever to rebuild, I uploaded the patch before it finished building and did not catch the build error :-(.
A new patch is coming.

[Yael]

Created attachment 50110 [details]
Once again, fix build in QWebPage.cpp.

[WebKit Review Bot]

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

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/HbLauncher/utils.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/urlloader.cpp:29:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/main.cpp:33:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.cpp"
WebKit/qt/HbLauncher/mainwindow.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/locationedit.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/qt/HbLauncher/webview.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.h"
WebKit/qt/HbLauncher/webpage.cpp:33:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 7 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 50110 [details]
Once again, fix build in QWebPage.cpp.

This should really be split up in separate patches. Also, we really want to avoid having yet another launcher if possible, not sharing any code at all.

[Chris Jerdonek]

(In reply to comment #8)
> (In reply to comment #7)
> > check-webkit-style may need some tweaking.  Please file bugs.  Also this does
> > not build on Qt.  Is that because the Qt bot is still 4.5 and this requires
> > 4.6?
> 
> I am not sure how to fix check-webkit-style for this specific problem. The
> affected files are the test application, and as such it should link only with
> public API, and should not include config,h.

FYI, there is an easy way to exempt certain file paths from the style check for one or more style categories.  This information is stored in a certain configuration variable.

Here's an example fix to see how:

https://bugs.webkit.org/show_bug.cgi?id=34604

[Simon Hausmann]

I'm still sceptical that we should solve this problem with a hard dependency to Orbit. Right now the build order of the software stack is: Qt, WebKit, Orbit. In fact right now Orbit has a dependency to QtWebKit.

We're looking at the styling of buttons, lineedits and the handling of popups. Especially for the latter the Maemo 5 team has demonstrated nicely that this can and should be solved within Qt, not above. The appearance of buttons and lineedits can be addressed using a Qt style.

If that Qt style really _needs_ to link against Orbit for some reason, then Orbit can provide that style and the Qt style plugin mechanism can handle it.

However I don't think at this point it makes sense for QtWebKit to link against a library further up in the software stack.

If we don't solve this with a QStyle then we can still think about introducing a plugin interface inside of WebKit for this purpose, but I think that should remain a last resort.

[Yael]

(In reply to comment #13)
> I'm still sceptical that we should solve this problem with a hard dependency to
> Orbit. Right now the build order of the software stack is: Qt, WebKit, Orbit.
> In fact right now Orbit has a dependency to QtWebKit.
> 
> We're looking at the styling of buttons, lineedits and the handling of popups.
> Especially for the latter the Maemo 5 team has demonstrated nicely that this
> can and should be solved within Qt, not above. The appearance of buttons and
> lineedits can be addressed using a Qt style.
> 
> If that Qt style really _needs_ to link against Orbit for some reason, then
> Orbit can provide that style and the Qt style plugin mechanism can handle it.
> 
> However I don't think at this point it makes sense for QtWebKit to link against
> a library further up in the software stack.
> 
> If we don't solve this with a QStyle then we can still think about introducing
> a plugin interface inside of WebKit for this purpose, but I think that should
> remain a last resort.
Orbit dependency on QtWebKit is only in the document loader, and from our discussion with the Orbit team I understand that they will remove this dependency.

The Orbit QStyle plugin is the right thing to do, but it still leaves us with at least 2 unsolved problems - 
1. The popup will not have the same effects as other Orbit dialogs.
2. Tooltips will not work.

[Simon Hausmann]

(In reply to comment #14)
> (In reply to comment #13)
> > I'm still sceptical that we should solve this problem with a hard dependency to
> > Orbit. Right now the build order of the software stack is: Qt, WebKit, Orbit.
> > In fact right now Orbit has a dependency to QtWebKit.
> > 
> > We're looking at the styling of buttons, lineedits and the handling of popups.
> > Especially for the latter the Maemo 5 team has demonstrated nicely that this
> > can and should be solved within Qt, not above. The appearance of buttons and
> > lineedits can be addressed using a Qt style.
> > 
> > If that Qt style really _needs_ to link against Orbit for some reason, then
> > Orbit can provide that style and the Qt style plugin mechanism can handle it.
> > 
> > However I don't think at this point it makes sense for QtWebKit to link against
> > a library further up in the software stack.
> > 
> > If we don't solve this with a QStyle then we can still think about introducing
> > a plugin interface inside of WebKit for this purpose, but I think that should
> > remain a last resort.
> Orbit dependency on QtWebKit is only in the document loader, and from our
> discussion with the Orbit team I understand that they will remove this
> dependency.
> 
> The Orbit QStyle plugin is the right thing to do, but it still leaves us with
> at least 2 unsolved problems - 
> 1. The popup will not have the same effects as other Orbit dialogs.

That's the current state. I wonder if it could be solved though, if the style knows about WebKit using the QGraphicsView.

> 2. Tooltips will not work.

Why do tooltips not work?

[Yael]

(In reply to comment #15)
> Why do tooltips not work?
My bad. They do work with the Orbitized QtLauncher.

[Yael]

An alternative solution is being worked on in https://bugs.webkit.org/show_bug.cgi?id=38438.

[Luiz Agostini]

(In reply to comment #17)
> An alternative solution is being worked on in
> https://bugs.webkit.org/show_bug.cgi?id=38438.

bug 38438 provides plugin support by QtWebKit for combo popup customizations. It has attached a very simple example that uses orbit to implement the popups. That patch will not provide any orbit related code. The focus there is just on the support of plugins by QtWebkit.

On the other hand it would be nice to have some feedback to know if the plugin interface provides what is needed for the orbit popus.