Bug 38324 - [Qt] Fix compilation with QT_NO_FEATURE
Summary: [Qt] Fix compilation with QT_NO_FEATURE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-04-29 06:22 PDT by Tasuku Suzuki
Modified: 2011-04-19 05:15 PDT (History)
8 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Tasuku Suzuki 2010-04-29 06:22:53 PDT
QtWebKit uses some Qt class without checking their availability.
Comment 1 Tasuku Suzuki 2010-04-29 06:30:45 PDT
Created attachment 54694 [details]
Patch to fix compilation with QT_NO_BEARERMANAGEMENT
Comment 2 Tasuku Suzuki 2010-04-29 08:50:48 PDT
Created attachment 54711 [details]
Patch to fix compilation with QT_NO_COMBOBOX
Comment 3 Tasuku Suzuki 2010-04-29 18:28:57 PDT
Created attachment 54768 [details]
Patch to fix compilation with QT_NO_GRAPHICSEFFECT
Comment 4 Laszlo Gombos 2010-04-29 20:39:54 PDT
(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
Comment 5 Tasuku Suzuki 2010-04-29 20:51:24 PDT
(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.
Comment 6 Laszlo Gombos 2010-04-29 21:08:00 PDT
> > > 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 Tasuku Suzuki 2010-04-30 08:09:53 PDT
Created attachment 54803 [details]
Patch to fix compilation with QT_NO_LINEEDIT
Comment 8 Eric Seidel (no email) 2010-04-30 11:55:24 PDT
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.
Comment 9 Tasuku Suzuki 2010-04-30 22:53:08 PDT
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.
Comment 10 Tasuku Suzuki 2010-05-01 01:01:14 PDT
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.
Comment 11 WebKit Commit Bot 2010-05-02 14:31:38 PDT
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>
Comment 12 WebKit Commit Bot 2010-05-02 15:19:06 PDT
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>
Comment 13 Kenneth Rohde Christiansen 2010-05-07 06:20:28 PDT
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?
Comment 14 Kenneth Rohde Christiansen 2010-05-07 06:22:55 PDT
No'am can you please look at the 'Patch to fix compilation with QT_NO_GRAPHICSEFFECT' patch?
Comment 15 Noam Rosenthal 2010-05-07 11:38:26 PDT
 Patch to fix compilation with QT_NO_GRAPHICSEFFECT looks good to me.
Comment 16 WebKit Commit Bot 2010-05-14 19:42:45 PDT
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
Comment 17 Adam Barth 2010-05-15 00:16:33 PDT
Please upload a new patch that applies cleanly to top of tree.
Comment 18 Tasuku Suzuki 2010-05-17 07:23:01 PDT
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.
Comment 19 WebKit Commit Bot 2010-05-17 10:43:38 PDT
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>
Comment 20 Tasuku Suzuki 2010-05-17 16:15:56 PDT
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.
Comment 21 Tasuku Suzuki 2010-05-19 22:09:07 PDT
Created attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES
Comment 22 Kenneth Rohde Christiansen 2010-05-20 05:42:32 PDT
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.
Comment 23 Tasuku Suzuki 2010-05-20 18:52:45 PDT
(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);
Comment 24 WebKit Commit Bot 2010-05-21 08:28:47 PDT
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>
Comment 25 Tasuku Suzuki 2010-05-24 07:43:57 PDT
Created attachment 56885 [details]
Patch to fix compilation with QT_NO_TEMPORARYFILE
Comment 26 WebKit Commit Bot 2010-05-24 22:03:55 PDT
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>
Comment 27 Eric Seidel (no email) 2010-06-02 02:26:10 PDT
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.
Comment 28 Shinichiro Hamaji 2010-06-02 23:46:40 PDT
Comment on attachment 56562 [details]
Patch to fix compilation with QT_NO_PROPERTIES

Looks sane to me
Comment 29 WebKit Commit Bot 2010-06-03 00:03:18 PDT
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>
Comment 30 WebKit Commit Bot 2010-06-03 00:16:22 PDT
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
Comment 31 Noam Rosenthal 2010-06-03 12:44:27 PDT
Created attachment 57804 [details]
Minor fix to QT_NO_GRAPHICSEFFECT opt-out in AC
Comment 32 Shinichiro Hamaji 2010-06-03 20:50:18 PDT
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
Comment 33 WebKit Commit Bot 2010-06-04 01:44:30 PDT
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>
Comment 34 WebKit Commit Bot 2010-06-04 01:44:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 35 Simon Hausmann 2010-06-07 01:56:57 PDT
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 Tasuku Suzuki 2010-06-21 19:19:03 PDT
(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 Simon Hausmann 2010-06-22 05:49:58 PDT
(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 Tasuku Suzuki 2010-08-14 00:36:48 PDT
Created attachment 64414 [details]
Patch to fix compilation with QT_NO_DATESTRING
Comment 39 Tasuku Suzuki 2010-08-14 00:37:36 PDT
I found a new compilation error with QT_NO_DATESTRING.
Comment 40 WebKit Commit Bot 2010-08-14 12:24:53 PDT
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>
Comment 41 WebKit Commit Bot 2010-08-14 12:25:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 42 Ademar Reis 2010-09-21 11:12:31 PDT
Revision r65371 cherry-picked into qtwebkit-2.1 with commit 0512e77 <http://gitorious.org/webkit/qtwebkit/commit/0512e77>