RESOLVED FIXED 64690
[Qt][WK2] Make QDesktopWebView::navigationAction method usable in QML.
https://bugs.webkit.org/show_bug.cgi?id=64690
Summary [Qt][WK2] Make QDesktopWebView::navigationAction method usable in QML.
Alexis Menard (darktears)
Reported 2011-07-18 06:02:45 PDT
[Qt][WK2] Implement load/back/forward/stop/reload for QDesktopWebView.
Attachments
Patch (2.34 KB, patch)
2011-07-18 06:05 PDT, Alexis Menard (darktears)
no flags
Patch (1.43 KB, patch)
2011-07-18 13:26 PDT, Alexis Menard (darktears)
benjamin: review+
benjamin: commit-queue-
Alexis Menard (darktears)
Comment 1 2011-07-18 06:05:37 PDT
WebKit Review Bot
Comment 2 2011-07-18 06:08:19 PDT
Attachment 101153 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:24: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alexis Menard (darktears)
Comment 3 2011-07-18 06:11:02 PDT
(In reply to comment #2) > Attachment 101153 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 > > Source/WebKit2/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:24: Alphabetical sorting problem. [build/include_order] [4] > Total errors found: 2 in 3 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. I believe it's good like that.
Benjamin Poulain
Comment 4 2011-07-18 06:29:45 PDT
Comment on attachment 101153 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101153&action=review + this needs tests :) >> Source/WebKit2/ChangeLog:6 >> + Just hook up to the QtWebPage proxy to trigger the appropriate actions. > > Line contains tab character. [whitespace/tab] [5] Uh :) > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:202 > +void QDesktopWebView::back() > +{ > + d->page.triggerAction(QtWebPageProxy::Back); > +} > + > +void QDesktopWebView::forward() > +{ > + d->page.triggerAction(QtWebPageProxy::Forward); > +} > + > +void QDesktopWebView::reload() > +{ > + d->page.triggerAction(QtWebPageProxy::Reload); > +} > + > +void QDesktopWebView::stop() > +{ > + d->page.triggerAction(QtWebPageProxy::Stop); > +} > + Those should call navigationAction(). > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:57 > + void back(); > + void forward(); > + void reload(); > + void stop(); I am not fan of this. I like the "goBack", "goForward" from Cocoa's WebView :) The stop, reload, back and forward action are all conditional to the state of the engine. Because of that QAction make much more sense than those slots.
Alexis Menard (darktears)
Comment 5 2011-07-18 12:56:34 PDT
I think in general we should make QDesktopWebView usable directly in QML. People should be able to do import QtWebKit 2.0 and get a ready to use QML element. Today we need to create a stupid wrapper just to QML_DECLARE_TYPE, forward the signals, add properties declarations and add the proper Q_INVOKABLE. I don't see why QDesktopWebView could not inherits from QDeclarativeItem directly (or whatever replacement comes with Qt Quick 2.0) because anyway our supported path in the future is QML, the C++ API being a nice fallback. Any thoughts?
Alexis Menard (darktears)
Comment 6 2011-07-18 13:26:56 PDT
Benjamin Poulain
Comment 7 2011-07-18 16:58:37 PDT
Comment on attachment 101196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101196&action=review > Source/WebKit2/ChangeLog:3 > + [Qt][WK2] Make QDesktopWebView QML friendly. You should have a more descriptive title for this.
Alexis Menard (darktears)
Comment 8 2011-07-19 04:07:05 PDT
Note You need to log in before you can comment on or make changes to this bug.