Bug 38324 – [Qt] Fix compilation with QT_NO_FEATURE

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Tasuku Suzuki]

QtWebKit uses some Qt class without checking their availability.

[Tasuku Suzuki]

Created attachment 54694 [details]
Patch to fix compilation with QT_NO_BEARERMANAGEMENT

[Tasuku Suzuki]

Created attachment 54711 [details]
Patch to fix compilation with QT_NO_COMBOBOX

[Tasuku Suzuki]

Created attachment 54768 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT

[Laszlo Gombos]

(In reply to comment #1)
> Created an attachment (id=54694) [details]
> Patch to fix compilation with QT_NO_BEARERMANAGEMENT

What about something like the following in WebCore/config.h ?

#ifdef QT_NO_BEARERMANAGEMENT
#undef ENABLE_QT_BEARER
#define ENABLE_QT_BEARER 0
#endif

[Tasuku Suzuki]

(In reply to comment #4)
> (In reply to comment #1)
> > Created an attachment (id=54694) [details] [details]
> > Patch to fix compilation with QT_NO_BEARERMANAGEMENT
> 
> What about something like the following in WebCore/config.h ?
> 
> #ifdef QT_NO_BEARERMANAGEMENT
> #undef ENABLE_QT_BEARER
> #define ENABLE_QT_BEARER 0
> #endif

This is what exactly I want. In order to make it work, we need the following as well. 
#if PLATFORM(QT)
#include <QtCore/qglobal.h>
#endif

If you are ok with this, I'll update the patch.

[Laszlo Gombos]

> > > Patch to fix compilation with QT_NO_BEARERMANAGEMENT

[...]

> > What about something like the following in WebCore/config.h ?

[...]

> This is what exactly I want. In order to make it work, we need the following as
> well. 
> #if PLATFORM(QT)
> #include <QtCore/qglobal.h>
> #endif
> 
> If you are ok with this, I'll update the patch.

You're right config.h does not work. 

Mayabe with a few changes we can do this either in NetworkStateNotifier.h or in NetworkStateNotifierPrivate.h - as the whole WebCore does not need to know about this rule, only the NetworkStateNotifier.

I understand that this pattern will only work for the BEARER_MGMT case, but that would already be a win.

[Tasuku Suzuki]

Created attachment 54803 [details]
Patch to fix compilation with QT_NO_LINEEDIT

[Eric Seidel (no email)]

Comment on attachment 54803 [details]
Patch to fix compilation with QT_NO_LINEEDIT

WebCore/platform/qt/RenderThemeQt.cpp:278
 +      return style->pixelMetric(QStyle::PM_DefaultFrameWidth, &opt, 0);
I prefer this style as:

#ifndef NO_LINE_EDIT
    QtLineEdit lineEdit = 0;
#else
   QtLineEdit* lineEdit = m_lineEdit;
#end
because then the long call doesn't have to be copied twice.

Otherwise this looks fine.  Even looks OK as is, I just prefer less copy/paste code.

[Tasuku Suzuki]

Created attachment 54847 [details]
Patch to fix compilation with QT_NO_LINEEDIT

(In reply to comment #8)
> (From update of attachment 54803 [details])
> WebCore/platform/qt/RenderThemeQt.cpp:278
>  +      return style->pixelMetric(QStyle::PM_DefaultFrameWidth, &opt, 0);
> I prefer this style as:
> 
> #ifndef NO_LINE_EDIT
>     QtLineEdit lineEdit = 0;
> #else
>    QtLineEdit* lineEdit = m_lineEdit;
> #end
> because then the long call doesn't have to be copied twice.
> 
> Otherwise this looks fine.  Even looks OK as is, I just prefer less copy/paste
> code.

Done.

[Tasuku Suzuki]

Created attachment 54848 [details]
Patch to fix compilation with QT_NO_BEARERMANAGEMENT

(In reply to comment #6)
> > > > Patch to fix compilation with QT_NO_BEARERMANAGEMENT
> 
> [...]
> 
> Mayabe with a few changes we can do this either in NetworkStateNotifier.h or in
> NetworkStateNotifierPrivate.h - as the whole WebCore does not need to know
> about this rule, only the NetworkStateNotifier.

I update the patch. Please take a look.

[WebKit Commit Bot]

Comment on attachment 54847 [details]
Patch to fix compilation with QT_NO_LINEEDIT

Clearing flags on attachment: 54847

Committed r58659: <http://trac.webkit.org/changeset/58659>

[WebKit Commit Bot]

Comment on attachment 54848 [details]
Patch to fix compilation with QT_NO_BEARERMANAGEMENT

Clearing flags on attachment: 54848

Committed r58662: <http://trac.webkit.org/changeset/58662>

[Kenneth Rohde Christiansen]

Comment on attachment 54711 [details]
Patch to fix compilation with QT_NO_COMBOBOX

It seems that it might be better to just not compile the QtFallbackWebPopup at all when comboboxes are not supported.

Luiz, comments please?

[Kenneth Rohde Christiansen]

No'am can you please look at the 'Patch to fix compilation with QT_NO_GRAPHICSEFFECT' patch?

[Noam Rosenthal]

 Patch to fix compilation with QT_NO_GRAPHICSEFFECT looks good to me.

[WebKit Commit Bot]

Comment on attachment 54768 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT

Rejecting patch 54768 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Simon Hausmann', u'--force']" exit_code: 1
Last 500 characters of output:

patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp
Hunk #1 succeeded at 43 (offset -3 lines).
Hunk #2 succeeded at 92 (offset -3 lines).
Hunk #3 succeeded at 181 with fuzz 1 (offset -5 lines).
Hunk #4 succeeded at 531 (offset 36 lines).
Hunk #5 succeeded at 539 with fuzz 1 (offset 36 lines).
Hunk #6 FAILED at 617.
1 out of 6 hunks FAILED -- saving rejects to file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/2266095

[Adam Barth]

Please upload a new patch that applies cleanly to top of tree.

[Tasuku Suzuki]

Created attachment 56243 [details]
Patch to fix compilation with QT_NO_COMBOBOX

(In reply to comment #13)
> (From update of attachment 54711 [details])
> It seems that it might be better to just not compile the QtFallbackWebPopup at all when comboboxes are not supported.

Done.

[WebKit Commit Bot]

Comment on attachment 56243 [details]
Patch to fix compilation with QT_NO_COMBOBOX

Clearing flags on attachment: 56243

Committed r59614: <http://trac.webkit.org/changeset/59614>

[Tasuku Suzuki]

Created attachment 56285 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT

(In reply to comment #17)
> Please upload a new patch that applies cleanly to top of tree.

Updated.

[Tasuku Suzuki]

Created attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES

[Kenneth Rohde Christiansen]

Comment on attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES


> @@ -93,7 +93,10 @@ void InspectorClientQt::openInspectorFrontend(WebCore::InspectorController*)
>      // Web inspector. This is used for SDK purposes. Please keep this hook
>      // around and don't remove it.
>      // https://bugs.webkit.org/show_bug.cgi?id=35340
> -    QUrl inspectorUrl = inspector->property("_q_inspectorUrl").toUrl();
> +    QUrl inspectorUrl;
> +#ifndef QT_NO_PROPERTIES
> +    inspectorUrl = inspector->property("_q_inspectorUrl").toUrl();
> +#endif
>      if (!inspectorUrl.isValid())
>          inspectorUrl = QUrl("qrc:/webkit/inspector/inspector.html");
>      inspectorView->page()->mainFrame()->load(inspectorUrl);

I think the the above situation you should quard more code, as the code that follows the inspectorUrl = is not useful without an actual url.

[Tasuku Suzuki]

(In reply to comment #22)
> (From update of attachment 56562 [details])
> 
> I think the the above situation you should quard more code, as the code that follows the inspectorUrl = is not useful without an actual url.

Do you mean we should do like below?

-    QUrl inspectorUrl = inspector->property("_q_inspectorUrl").toUrl();
+    QUrl inspectorUrl;
+#ifndef QT_NO_PROPERTIES
+    inspectorUrl = inspector->property("_q_inspectorUrl").toUrl();
     if (!inspectorUrl.isValid())
+#endif
         inspectorUrl = QUrl("qrc:/webkit/inspector/inspector.html");
     inspectorView->page()->mainFrame()->load(inspectorUrl);

[WebKit Commit Bot]

Comment on attachment 56285 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT

Clearing flags on attachment: 56285

Committed r59934: <http://trac.webkit.org/changeset/59934>

[Tasuku Suzuki]

Created attachment 56885 [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE

[WebKit Commit Bot]

Comment on attachment 56885 [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE

Clearing flags on attachment: 56885

Committed r60128: <http://trac.webkit.org/changeset/60128>

[Eric Seidel (no email)]

Comment on attachment 54768 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT

Cleared Simon Hausmann's review+ from obsolete attachment 54768 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Shinichiro Hamaji]

Comment on attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES

Looks sane to me

[WebKit Commit Bot]

Comment on attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES

Clearing flags on attachment: 56562

Committed r60609: <http://trac.webkit.org/changeset/60609>

[WebKit Commit Bot]

Comment on attachment 56885 [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE

Rejecting patch 56885 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Shinichiro Hamaji', u'--force']" exit_code: 1
Parsed 2 diffs from patch file(s).
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/platform/qt/FileSystemQt.cpp
Hunk #1 FAILED at 117.
Hunk #2 FAILED at 125.
2 out of 2 hunks FAILED -- saving rejects to file WebCore/platform/qt/FileSystemQt.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/3008008

[Noam Rosenthal]

Created attachment 57804 [details]
Minor fix to QT_NO_GRAPHICSEFFECT opt-out in AC

[Shinichiro Hamaji]

Comment on attachment 56885 [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE

Clearing review flag as this patch has already been landed in http://trac.webkit.org/changeset/60128

[WebKit Commit Bot]

Comment on attachment 57804 [details]
Minor fix to QT_NO_GRAPHICSEFFECT opt-out in AC

Clearing flags on attachment: 57804

Committed r60665: <http://trac.webkit.org/changeset/60665>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Revision r58659 cherry-picked into qtwebkit-2.0 with commit 956d4e62edaa10351aca73f7ff006382d4a49794
Revision r58662 cherry-picked into qtwebkit-2.0 with commit 62851307ebf1f7120c077fe505a41d78306d56af
Revision r59614 cherry-picked into qtwebkit-2.0 with commit 91bdb85e57ca83bb804d89cb0a907096bcac129a
Revision r59934 cherry-picked into qtwebkit-2.0 with commit ab3e5c73b851b001edfe4ffb74470a4caebdaae5
Revision r60128 cherry-picked into qtwebkit-2.0 with commit 4675614b0560ed602f069620f39de5800c9aef55
Revision r60609 cherry-picked into qtwebkit-2.0 with commit c2150bf1881b8d3dda64234d09db99a0207ba20c
Revision r60665 cherry-picked into qtwebkit-2.0 with commit 2f701f79000ddc830d39039ee814ccf08c26b05a

[Tasuku Suzuki]

(In reply to comment #35)
> Revision r59614 cherry-picked into qtwebkit-2.0 with commit 91bdb85e57ca83bb804d89cb0a907096bcac129a

The patch for QT_NO_COMBOBOX depends on the patch 1 in https://bugs.webkit.org/show_bug.cgi?id=38438 which is not merged into the 2.0 release branch. 4.7 branch in Qt fails to compile.

What can I do for this?

[Simon Hausmann]

(In reply to comment #36)
> (In reply to comment #35)
> > Revision r59614 cherry-picked into qtwebkit-2.0 with commit 91bdb85e57ca83bb804d89cb0a907096bcac129a
> 
> The patch for QT_NO_COMBOBOX depends on the patch 1 in https://bugs.webkit.org/show_bug.cgi?id=38438 which is not merged into the 2.0 release branch. 4.7 branch in Qt fails to compile.
> 
> What can I do for this?

Could you send me a patch for it?

Thanks :)

[Tasuku Suzuki]

Created attachment 64414 [details]
Patch to fix compilation with QT_NO_DATESTRING

[Tasuku Suzuki]

I found a new compilation error with QT_NO_DATESTRING.

[WebKit Commit Bot]

Comment on attachment 64414 [details]
Patch to fix compilation with QT_NO_DATESTRING

Clearing flags on attachment: 64414

Committed r65371: <http://trac.webkit.org/changeset/65371>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ademar Reis]

Revision r65371 cherry-picked into qtwebkit-2.1 with commit 0512e77 <http://gitorious.org/webkit/qtwebkit/commit/0512e77>