[webkit-reviews] review denied: [Bug 34902] [Qt] Add methods to QtWebKit API for setting and consulting the CSS2 media type : [Attachment 56110] Patch (Add as a setting)

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 34902: [Qt] Add methods to QtWebKit API for setting and consulting the CSS2
media type
https://bugs.webkit.org/show_bug.cgi?id=34902

Attachment 56110: Patch (Add as a setting)
https://bugs.webkit.org/attachment.cgi?id=56110&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
WebCore/page/Settings.h:311
 +	    Page* page() { return m_page; }
This should probably be const.

WebKit/qt/Api/qwebframe.cpp:225
 +  void QWebFramePrivate::applyMediaType(WebCore::Frame* frame, const
WebCore::String& mediaType, bool recursive)
I don't see where false is passed from any caller of this method, so I think
you can remove the argument and call the function applyMediaTypeRecursively
(Webkittish ;)

WebKit/qt/Api/qwebsettings.cpp:584
 +  void QWebSettings::setStyleSheetMediaType(const QString& mediaType)
The implementation of this function should fall back to the media type of the
global QWebSettings object.

The usual pattern is to do the changes in apply(), which will also be be called
if you change settings in the global QWebSettings object.

WebKit/qt/Api/qwebsettings.cpp:591
 +  QString QWebSettings::styleSheetMediaType() const
I think the implementation of this function should returnd->styleSheetMediaType
straight, instead of automatically falling back.

See the same pattern for example with QWebSettings::userStyleSheetUrl and
setUserStyleSheetUrl.