Bug 38324 - [Qt] Fix compilation with QT_NO_FEATURE
: [Qt] Fix compilation with QT_NO_FEATURE
Status: RESOLVED FIXED
: WebKit
WebKit Qt
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
: Qt, QtTriaged
:
:
  Show dependency treegraph
 
Reported: 2010-04-29 06:22 PST by
Modified: 2011-04-19 05:15 PST (History)


Attachments
Patch to fix compilation with QT_NO_BEARERMANAGEMENT (5.13 KB, patch)
2010-04-29 06:30 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_COMBOBOX (4.46 KB, patch)
2010-04-29 08:50 PST, Tasuku Suzuki
kenneth: review-
kenneth: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_GRAPHICSEFFECT (3.45 KB, patch)
2010-04-29 18:28 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_LINEEDIT (1.49 KB, patch)
2010-04-30 08:09 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_LINEEDIT (1.54 KB, patch)
2010-04-30 22:53 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_BEARERMANAGEMENT (2.52 KB, patch)
2010-05-01 01:01 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_COMBOBOX (2.13 KB, patch)
2010-05-17 07:23 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_GRAPHICSEFFECT (2.99 KB, patch)
2010-05-17 16:15 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_PROPERTIES (4.13 KB, patch)
2010-05-19 22:09 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_TEMPORARYFILE (1.28 KB, patch)
2010-05-24 07:43 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff
Minor fix to QT_NO_GRAPHICSEFFECT opt-out in AC (1.83 KB, patch)
2010-06-03 12:44 PST, Noam Rosenthal
no flags Review Patch | Details | Formatted Diff | Diff
Patch to fix compilation with QT_NO_DATESTRING (1.41 KB, patch)
2010-08-14 00:36 PST, Tasuku Suzuki
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-04-29 06:22:53 PST
QtWebKit uses some Qt class without checking their availability.
------- Comment #1 From 2010-04-29 06:30:45 PST -------
Created an attachment (id=54694) [details]
Patch to fix compilation with QT_NO_BEARERMANAGEMENT
------- Comment #2 From 2010-04-29 08:50:48 PST -------
Created an attachment (id=54711) [details]
Patch to fix compilation with QT_NO_COMBOBOX
------- Comment #3 From 2010-04-29 18:28:57 PST -------
Created an attachment (id=54768) [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT
------- Comment #4 From 2010-04-29 20:39:54 PST -------
(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
------- Comment #5 From 2010-04-29 20:51:24 PST -------
(In reply to comment #4)
> (In reply to comment #1)
> > Created an attachment (id=54694) [details] [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.
------- Comment #6 From 2010-04-29 21:08:00 PST -------
> > > 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.
------- Comment #7 From 2010-04-30 08:09:53 PST -------
Created an attachment (id=54803) [details]
Patch to fix compilation with QT_NO_LINEEDIT
------- Comment #8 From 2010-04-30 11:55:24 PST -------
(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.
------- Comment #9 From 2010-04-30 22:53:08 PST -------
Created an attachment (id=54847) [details]
Patch to fix compilation with QT_NO_LINEEDIT

(In reply to comment #8)
> (From update of attachment 54803 [details] [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.
------- Comment #10 From 2010-05-01 01:01:14 PST -------
Created an attachment (id=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.
------- Comment #11 From 2010-05-02 14:31:38 PST -------
(From update of attachment 54847 [details])
Clearing flags on attachment: 54847

Committed r58659: <http://trac.webkit.org/changeset/58659>
------- Comment #12 From 2010-05-02 15:19:06 PST -------
(From update of attachment 54848 [details])
Clearing flags on attachment: 54848

Committed r58662: <http://trac.webkit.org/changeset/58662>
------- Comment #13 From 2010-05-07 06:20:28 PST -------
(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.

Luiz, comments please?
------- Comment #14 From 2010-05-07 06:22:55 PST -------
No'am can you please look at the 'Patch to fix compilation with QT_NO_GRAPHICSEFFECT' patch?
------- Comment #15 From 2010-05-07 11:38:26 PST -------
 Patch to fix compilation with QT_NO_GRAPHICSEFFECT looks good to me.
------- Comment #16 From 2010-05-14 19:42:45 PST -------
(From update of attachment 54768 [details])
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
------- Comment #17 From 2010-05-15 00:16:33 PST -------
Please upload a new patch that applies cleanly to top of tree.
------- Comment #18 From 2010-05-17 07:23:01 PST -------
Created an attachment (id=56243) [details]
Patch to fix compilation with QT_NO_COMBOBOX

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

Done.
------- Comment #19 From 2010-05-17 10:43:38 PST -------
(From update of attachment 56243 [details])
Clearing flags on attachment: 56243

Committed r59614: <http://trac.webkit.org/changeset/59614>
------- Comment #20 From 2010-05-17 16:15:56 PST -------
Created an attachment (id=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.
------- Comment #21 From 2010-05-19 22:09:07 PST -------
Created an attachment (id=56562) [details]
Patch to fix compilation with QT_NO_PROPERTIES
------- Comment #22 From 2010-05-20 05:42:32 PST -------
(From update of attachment 56562 [details])

> @@ -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.
------- Comment #23 From 2010-05-20 18:52:45 PST -------
(In reply to comment #22)
> (From update of attachment 56562 [details] [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);
------- Comment #24 From 2010-05-21 08:28:47 PST -------
(From update of attachment 56285 [details])
Clearing flags on attachment: 56285

Committed r59934: <http://trac.webkit.org/changeset/59934>
------- Comment #25 From 2010-05-24 07:43:57 PST -------
Created an attachment (id=56885) [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE
------- Comment #26 From 2010-05-24 22:03:55 PST -------
(From update of attachment 56885 [details])
Clearing flags on attachment: 56885

Committed r60128: <http://trac.webkit.org/changeset/60128>
------- Comment #27 From 2010-06-02 02:26:10 PST -------
(From update of attachment 54768 [details])
Cleared Simon Hausmann's review+ from obsolete attachment 54768 [details] so that this bug does not appear in http://webkit.org/pending-commit.
------- Comment #28 From 2010-06-02 23:46:40 PST -------
(From update of attachment 56562 [details])
Looks sane to me
------- Comment #29 From 2010-06-03 00:03:18 PST -------
(From update of attachment 56562 [details])
Clearing flags on attachment: 56562

Committed r60609: <http://trac.webkit.org/changeset/60609>
------- Comment #30 From 2010-06-03 00:16:22 PST -------
(From update of attachment 56885 [details])
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
------- Comment #31 From 2010-06-03 12:44:27 PST -------
Created an attachment (id=57804) [details]
Minor fix to QT_NO_GRAPHICSEFFECT opt-out in AC
------- Comment #32 From 2010-06-03 20:50:18 PST -------
(From update of attachment 56885 [details])
Clearing review flag as this patch has already been landed in http://trac.webkit.org/changeset/60128
------- Comment #33 From 2010-06-04 01:44:30 PST -------
(From update of attachment 57804 [details])
Clearing flags on attachment: 57804

Committed r60665: <http://trac.webkit.org/changeset/60665>
------- Comment #34 From 2010-06-04 01:44:38 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #35 From 2010-06-07 01:56:57 PST -------
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
------- Comment #36 From 2010-06-21 19:19:03 PST -------
(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?
------- Comment #37 From 2010-06-22 05:49:58 PST -------
(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 :)
------- Comment #38 From 2010-08-14 00:36:48 PST -------
Created an attachment (id=64414) [details]
Patch to fix compilation with QT_NO_DATESTRING
------- Comment #39 From 2010-08-14 00:37:36 PST -------
I found a new compilation error with QT_NO_DATESTRING.
------- Comment #40 From 2010-08-14 12:24:53 PST -------
(From update of attachment 64414 [details])
Clearing flags on attachment: 64414

Committed r65371: <http://trac.webkit.org/changeset/65371>
------- Comment #41 From 2010-08-14 12:25:00 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #42 From 2010-09-21 11:12:31 PST -------
Revision r65371 cherry-picked into qtwebkit-2.1 with commit 0512e77 <http://gitorious.org/webkit/qtwebkit/commit/0512e77>