[webkit-reviews] review denied: [Bug 73560] [Qt] [WK2] Support customizing popup menus with QML : [Attachment 118561] Patch

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied Caio Marcelo de Oliveira Filho
<cmarcelo at webkit.org>'s request for review:
Bug 73560: [Qt] [WK2] Support customizing popup menus with QML
https://bugs.webkit.org/show_bug.cgi?id=73560

Attachment 118561: Patch
https://bugs.webkit.org/attachment.cgi?id=118561&action=review

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118561&action=review


> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_itemSelector.qml:9

> +    property int shouldSelectIndexFirst

This sounds like a bool. can you find a better name?

>
Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_itemSelector.qml:10
> +    property int shouldSelectIndex

same with this

>
Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_itemSelector.qml:21
> +		      
selectorModel.items.select(WebView.view.shouldSelectIndexFirst)

Now that we've split up the selector model and the items model, you can name
the selector model "model"

>
Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_itemSelector.qml:24
> +		       selectorModel.items.reject()

reject() and accept() should be on the popup model, not on the individual items