Bug 70141 – [Qt][WK2] WebPreferences are impossible to use in QML.

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Alexis Menard (darktears)]

[Qt][WK2] WebPreferences are impossible to use in QML.

[Alexis Menard (darktears)]

Created attachment 111071 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 111071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111071&action=review

Some nits

> Source/WebKit2/ChangeLog:8
> +        Make possible to use qwkpreferences in QML. I renamed it

Make *it*. Don't use I. Renamed it to follow our new style regarding class names.

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:301
> +
> +

Double newline

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:60
> +    Q_PROPERTY(QWebPreferences* preferences READ preferences CONSTANT FINAL)

is preferences the right word to use in QML or should it be settings?

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:29
> +
> +

im not sure we use double newlines here

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:62
> +void QWebPreferencesPrivate::setAttribute(QWebPreferencesPrivate::WebAttribute attr, bool on)

I would have used "enable" and not "on".

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:94
> +
> +

double newline

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:153
> +
> +

again!

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:171
> +int QWebPreferencesPrivate::fontSize(QWebPreferencesPrivate::FontSize type) const

unsigned? Can we have negative sizes?

Wha? FontSize type? :-) s/FontSize/FontSizeType/ ? Kind?

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:251
> +bool QWebPreferences::XSSAuditingEnabled() const

xssAuditing...

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:355
> +void QWebPreferences::setMinimumFontSize(int size)

In the future we need someting like this per column width (-webkit-text-adjust)

> Source/WebKit2/UIProcess/API/qt/qwebpreferences.h:21
> +#ifndef QWEBREFERENCES_H
> +#define QWEBPREFERENCES_H

Something is wrong here! Misses P!

[Alexis Menard (darktears)]

(In reply to comment #2)
> (From update of attachment 111071 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111071&action=review
> 
> Some nits
> 
> > Source/WebKit2/ChangeLog:8
> > +        Make possible to use qwkpreferences in QML. I renamed it
> 
> Make *it*. Don't use I. Renamed it to follow our new style regarding class names.
> 
> > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:301
> > +
> > +
> 
> Double newline
> 
> > Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:60
> > +    Q_PROPERTY(QWebPreferences* preferences READ preferences CONSTANT FINAL)
> 
> is preferences the right word to use in QML or should it be settings?

I don't have a strong opinion here.

> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:29
> > +
> > +
> 
> im not sure we use double newlines here
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:62
> > +void QWebPreferencesPrivate::setAttribute(QWebPreferencesPrivate::WebAttribute attr, bool on)
> 
> I would have used "enable" and not "on".
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:94
> > +
> > +
> 
> double newline
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:153
> > +
> > +
> 
> again!
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:171
> > +int QWebPreferencesPrivate::fontSize(QWebPreferencesPrivate::FontSize type) const
> 
> unsigned? Can we have negative sizes?
> 
> Wha? FontSize type? :-) s/FontSize/FontSizeType/ ? Kind?
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:251
> > +bool QWebPreferences::XSSAuditingEnabled() const
> 
> xssAuditing...
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.cpp:355
> > +void QWebPreferences::setMinimumFontSize(int size)
> 
> In the future we need someting like this per column width (-webkit-text-adjust)
> 
> > Source/WebKit2/UIProcess/API/qt/qwebpreferences.h:21
> > +#ifndef QWEBREFERENCES_H
> > +#define QWEBPREFERENCES_H
> 
> Something is wrong here! Misses P!

[Alexis Menard (darktears)]

Created attachment 111246 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 111246 [details]
Patch

Don't we want the preferences in experimental so far?

[Alexis Menard (darktears)]

(In reply to comment #5)
> (From update of attachment 111246 [details])
> Don't we want the preferences in experimental so far?

Well I cleaned the flags and let the one that I feel "mandatory"

Of course WebGL, DeveloperExtras, and whatever will go in the experimental.

[WebKit Review Bot]

Comment on attachment 111246 [details]
Patch

Clearing flags on attachment: 111246

Committed r97740: <http://trac.webkit.org/changeset/97740>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.