[webkit-reviews] review denied: [Bug 72893] [QT][WK2] Add (experimental) viewport attributes view to MiniBrowser/qt : [Attachment 116191] Proposed patch

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied	review:
Bug 72893: [QT][WK2] Add (experimental) viewport attributes view to
MiniBrowser/qt
https://bugs.webkit.org/show_bug.cgi?id=72893

Attachment 116191: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=116191&action=review

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


> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:168
> +    Q_PROPERTY(QWebViewportAttributes* attributes READ attributes CONSTANT
FINAL)

I think viewportAttributes is better, since attributes is a bit too generic.

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:60
> +	   viewportAttributesItem.initialScale =
webView.experimental.attributes.initialScale
> +	   viewportAttributesItem.minimumScale =
webView.experimental.attributes.minimumScale
> +	   viewportAttributesItem.maximumScale =
webView.experimental.attributes.maximumScale
> +
> +	   viewportAttributesItem.isScalable =
webView.experimental.attributes.isScalable
> +	   viewportAttributesItem.layoutWidth =
webView.experimental.attributes.layoutWidth
> +	   viewportAttributesItem.layoutHeight =
webView.experimental.attributes.layoutHeight
> +    }

Why are you not using bindings for these?

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:242
> +	   onHeightChanged: updateViewportAttributes()
> +	   onWidthChanged: updateViewportAttributes()
> +	   onLoadSucceeded: updateViewportAttributes()

The viewport attributes themselves should change in response to these events,
and the bindings then updated.

> Tools/MiniBrowser/qt/qml/ViewportAttributesItem.qml:5
> +    color: "#000000"

black

> Tools/MiniBrowser/qt/qml/ViewportAttributesItem.qml:19
> +	   color: "#ffffff"

white

> Tools/MiniBrowser/qt/qml/ViewportAttributesItem.qml:25
> +	  color: "#ffffff"

white

> Tools/MiniBrowser/qt/qml/ViewportAttributesItem.qml:31
> +	  color: "#ffffff"

style