Bug 82195 – [Qt] Fix includes after QtDeclarative -> QtQML renaming

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

After QtDeclarative was renamed to QtQML in Qt5, we get warnings about it during 
building QtWebKit. That's why Simon disabled -Werror for Qt5-Webkit builds.

It's time to fix includes in WebKit and enable -Werror again.
( https://lists.webkit.org/pipermail/webkit-qt/2012-March/002586.html )

Ádám started working on it, patch is coming soon.

[Ádám Kallai]

Created attachment 134058 [details]
Build fix patch draft

Replaced QtDeclarative with QtQml with the help of the script found at:
https://lists.webkit.org/pipermail/webkit-qt/2012-March/002586.html

I only modified the parts that caused warnings while building:
"#warning Header <QtDeclarative/qdeclarativelist.h> is deprecated. Please include <QtQml/qqmllist.h> instead."

I would like to know if it needs to be replaced everywhere in Webkit Source or just at the most crucial parts. I tested it on Qt-4.8 and Qt-5.0 and the build was successful. 
I'm not sure if it's the best solution. Any idea and help would be appreciated.

[Csaba Osztrogonác]

Comment on attachment 134058 [details]
Build fix patch draft

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

And could you enable -Werror with this patch? ( http://trac.webkit.org/changeset/110422/trunk/Tools/qmake/mkspecs/features/unix/default_post.prf )
If you enable it, and click on "Submit for EWS analysis" we can see if it really works with Qt 4.8 and Qt 5.0 too. ;)

> Source/WebKit/qt/declarative/plugin.cpp:20
> +#include "qglobal.h"

What about including config.h instead of qglobal.h?

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:27
> +//#include <QtDeclarative/qdeclarativelist.h>

Remove this commented line.

[Ádám Kallai]

Created attachment 134577 [details]
proposed patch

I didn't notice unnecessary line in my Previous patch.
If I use 'config.h' then WebKit does not build with Qt5 so I need to use 'qglobal.h'.

I enabled -Werr in this patch.

[Early Warning System Bot]

Comment on attachment 134577 [details]
proposed patch

Attachment 134577 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12214019

[Csaba Osztrogonác]

(In reply to comment #4)
> (From update of attachment 134577 [details])
> Attachment 134577 [details] did not pass qt-wk2-ews (qt):
> Output: http://queues.webkit.org/results/12214019

OMG, I was sure if it builds for me. And now I notice that I missed to update Qt5 on the Qt-WK2 EWS. I'm doing it immediately.

[Csaba Osztrogonác]

Created attachment 134593 [details]
proposed patch

same as 134577, let the Qt-WK2 EWS be green :)

[Simon Hausmann]

Comment on attachment 134593 [details]
proposed patch

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

r=me. I'll let Ossy set cq+ for a safe landing :)

> Source/WebKit2/UIProcess/API/qt/qwebnavigationhistory_p.h:34
> -#include <qdeclarative.h>
> +#include <QtQml/qqml.h>

This is more of a nitpick, but there is no reason to use #include <ModuleName/class-or-header.h> in private header files or cpp files. There is a reason for adding the "redundant" module name in public header files, but for private header files and cpp files we can rely on the presence of QT += module-name in the .pro file and therefore use the shorter #include <ClassName> include style.

[Csaba Osztrogonác]

Comment on attachment 134593 [details]
proposed patch

cq- to let Ádám fix what Simon mentioned before landing.

[Kristóf Kosztyó]

patch landed in r112651 http://trac.webkit.org/changeset/112651

[Csaba Osztrogonác]

Reopen, because I had to roll it out. Unfortunately we had to go back to the previous Qt5 and it doesn't work with it.

[Csaba Osztrogonác]

Qt5 is updated, so I relanded it - http://trac.webkit.org/changeset/113330