Summary: | [WK2][Qt] Color chooser API missing | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Thiago Marcos P. Santos <tmpsantos> | ||||||||||
Component: | WebKit Qt | Assignee: | Thiago Marcos P. Santos <tmpsantos> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cmarcelo, dinu.jacob, hausmann, kenneth, menard, vestbo, webkit.review.bot, zoltan | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 87495 | ||||||||||||
Bug Blocks: | 87988, 87989, 88101 | ||||||||||||
Attachments: |
|
Description
Thiago Marcos P. Santos
2012-05-29 08:32:59 PDT
Created attachment 145138 [details]
Patch
Looks OK to me, but cc'ing Tor Arne to look at the QML related code. Comment on attachment 145138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145138&action=review Nice stuff, thanks ! > Source/WebKit2/Target.pri:801 > +contains(DEFINES, ENABLE_INPUT_TYPE_COLOR=1) { Maybe we could enable it by default for Qt 5 in Tools/qmake/mkspecs/features/features.prf ? > Source/WebKit2/UIProcess/qt/WebColorChooserProxyQt.cpp:36 > + Q_PROPERTY(QColor currentColor READ currentColor CONSTANT FINAL) Maybe we'd want to expose the elementRect like in WebPopupMenuProxyQt. (In reply to comment #3) > (From update of attachment 145138 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145138&action=review > > Nice stuff, thanks ! Thank you very much for having a look. > > > Source/WebKit2/Target.pri:801 > > +contains(DEFINES, ENABLE_INPUT_TYPE_COLOR=1) { > > Maybe we could enable it by default for Qt 5 in Tools/qmake/mkspecs/features/features.prf ? Good idea. Gonna ask on the qt webkit list when I get this landed if it is fine to enable by default since IMO, the feature is complete. > > > Source/WebKit2/UIProcess/qt/WebColorChooserProxyQt.cpp:36 > > + Q_PROPERTY(QColor currentColor READ currentColor CONSTANT FINAL) > > Maybe we'd want to expose the elementRect like in WebPopupMenuProxyQt. The problem here is, unlike PopupMenu, WebCore doesn't provide the coordinates. :/ Created attachment 147286 [details]
Patch
Rebased.
Comment on attachment 147286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147286&action=review Can we have a tesT? > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1158 > + if (d->colorChooser == colorChooser) > + return; we normally add newline after return (In reply to comment #6) > (From update of attachment 147286 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147286&action=review > > Can we have a tesT? You do have one. Check bug 88101. :) > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1158 > > + if (d->colorChooser == colorChooser) > > + return; > > we normally add newline after return Gonna fix it. Thanks for reviewing. Created attachment 147302 [details]
Patch v2
Created attachment 147547 [details]
Patch
Rebased.
Comment on attachment 147547 [details] Patch Clearing flags on attachment: 147547 Committed r120915: <http://trac.webkit.org/changeset/120915> All reviewed patches have been landed. Closing bug. |