Bug 40918 - [Qt] QtDeclarative Webview element has a fixed white background
Summary: [Qt] QtDeclarative Webview element has a fixed white background
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P4 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-06-21 08:14 PDT by Alan Alpert
Modified: 2011-05-09 13:06 PDT (History)
13 users (show)

See Also:


Attachments
Proposed patch to implement the feature. (6.03 KB, patch)
2011-02-28 14:14 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
With a changelog it's better (7.30 KB, patch)
2011-02-28 14:16 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Make it build if Qt < 4.7.2 (7.70 KB, patch)
2011-03-01 04:50 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
The ifdef trick can't work moc can't parse the Q_PROPERTY anyway because moc ignore the ifdef directives. (7.30 KB, patch)
2011-03-01 05:51 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
#if QT_VERSION >= 0x040702 try. (7.53 KB, patch)
2011-03-03 06:23 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
fix Qt SDK 4.7.3 build bust due to Q_REVISION being undefined (4.10 KB, patch)
2011-04-25 15:04 PDT, Siddharth Mathur
no flags Details | Formatted Diff | Diff
fix indent in Changelog (4.11 KB, patch)
2011-04-25 15:07 PDT, Siddharth Mathur
kling: review+
kling: commit-queue-
Details | Formatted Diff | Diff
Patch (3.76 KB, patch)
2011-04-28 17:46 PDT, Keith Kyzivat
no flags Details | Formatted Diff | Diff
Patch (3.58 KB, patch)
2011-04-28 19:32 PDT, Keith Kyzivat
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Alpert 2010-06-21 08:14:57 PDT
It should have a property to set the background color to transparent or another color (although the webkit default of white is fine)

Moved from Qt tracker where it was http://bugreports.qt.nokia.com/browse/QTBUG-11591
Comment 1 Krzysztof Klinikowski 2011-01-30 15:07:46 PST
Any progress with that bug?
Comment 2 Alexis Menard (darktears) 2011-02-28 14:14:06 PST
Created attachment 84117 [details]
Proposed patch to implement the feature.
Comment 3 Alexis Menard (darktears) 2011-02-28 14:16:47 PST
Created attachment 84118 [details]
With a changelog it's better
Comment 4 Early Warning System Bot 2011-02-28 17:35:52 PST
Attachment 84118 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8077215
Comment 5 Alexis Menard (darktears) 2011-03-01 04:50:14 PST
Created attachment 84212 [details]
Make it build if Qt < 4.7.2

The Q_REVISION feature was added for Qt 4.7.2
Comment 6 Early Warning System Bot 2011-03-01 05:14:09 PST
Attachment 84212 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8070699
Comment 7 Alexis Menard (darktears) 2011-03-01 05:51:58 PST
Created attachment 84219 [details]
The ifdef trick can't work moc can't parse the Q_PROPERTY anyway because moc ignore the ifdef directives.

We will just wait a bit to integrate it, but the review is still welcome.
Comment 8 Early Warning System Bot 2011-03-01 06:19:09 PST
Attachment 84219 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8074584
Comment 9 Simon Hausmann 2011-03-02 00:57:45 PST
(In reply to comment #5)
> Created an attachment (id=84212) [details]
> Make it build if Qt < 4.7.2
> 
> The Q_REVISION feature was added for Qt 4.7.2

I think we should keep the trunk building with Qt 4.7.0 and 4.7.1.

Moc _does_ respect #ifdefs. It's just that _some_ more complex ones, such as WebKit's #if USE(FOO) it can't, but a simple #if QT_VERSION >= 0x040702 it does support.
Comment 10 Alexis Menard (darktears) 2011-03-02 03:15:02 PST
(In reply to comment #9)
> (In reply to comment #5)
> > Created an attachment (id=84212) [details] [details]
> > Make it build if Qt < 4.7.2
> > 
> > The Q_REVISION feature was added for Qt 4.7.2
> 
> I think we should keep the trunk building with Qt 4.7.0 and 4.7.1.
> 
> Moc _does_ respect #ifdefs. It's just that _some_ more complex ones, such as WebKit's #if USE(FOO) it can't, but a simple #if QT_VERSION >= 0x040702 it does support.


No see : "Make it build if Qt < 4.7.2" patch. Moc fail to parse the Q_PROPERTY and try to parse it even inside the ifdef :(.
Comment 11 Simon Hausmann 2011-03-03 05:59:41 PST
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #5)
> > > Created an attachment (id=84212) [details] [details] [details]
> > > Make it build if Qt < 4.7.2
> > > 
> > > The Q_REVISION feature was added for Qt 4.7.2
> > 
> > I think we should keep the trunk building with Qt 4.7.0 and 4.7.1.
> > 
> > Moc _does_ respect #ifdefs. It's just that _some_ more complex ones, such as WebKit's #if USE(FOO) it can't, but a simple #if QT_VERSION >= 0x040702 it does support.
> 
> 
> No see : "Make it build if Qt < 4.7.2" patch. Moc fail to parse the Q_PROPERTY and try to parse it even inside the ifdef :(.

Yes, because the #ifdef looks like this:

#if QT_VERSION >= QT_VERSION_CHECK(4, 7, 2)

which unfortunately doesn't work with moc as moc doesn't expand macros with
function style arguments.

However if you write

#if QT_VERSION >= 0x040702

then moc will get it right. (high chance :)
Comment 12 Alexis Menard (darktears) 2011-03-03 06:09:49 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #5)
> > > > Created an attachment (id=84212) [details] [details] [details] [details]
> > > > Make it build if Qt < 4.7.2
> > > > 
> > > > The Q_REVISION feature was added for Qt 4.7.2
> > > 
> > > I think we should keep the trunk building with Qt 4.7.0 and 4.7.1.
> > > 
> > > Moc _does_ respect #ifdefs. It's just that _some_ more complex ones, such as WebKit's #if USE(FOO) it can't, but a simple #if QT_VERSION >= 0x040702 it does support.
> > 
> > 
> > No see : "Make it build if Qt < 4.7.2" patch. Moc fail to parse the Q_PROPERTY and try to parse it even inside the ifdef :(.
> 
> Yes, because the #ifdef looks like this:
> 
> #if QT_VERSION >= QT_VERSION_CHECK(4, 7, 2)
> 
> which unfortunately doesn't work with moc as moc doesn't expand macros with
> function style arguments.
> 
> However if you write
> 
> #if QT_VERSION >= 0x040702
> 
> then moc will get it right. (high chance :)

On my way to try :)
Comment 13 Alexis Menard (darktears) 2011-03-03 06:23:20 PST
Created attachment 84551 [details]
#if QT_VERSION >= 0x040702 try.

Let see with that one.
Comment 14 Andreas Kling 2011-03-10 04:51:26 PST
Comment on attachment 84551 [details]
#if QT_VERSION >= 0x040702 try.

LGTM.
Comment 15 WebKit Commit Bot 2011-03-10 15:36:45 PST
Comment on attachment 84551 [details]
#if QT_VERSION >= 0x040702 try.

Clearing flags on attachment: 84551

Committed r80774: <http://trac.webkit.org/changeset/80774>
Comment 16 WebKit Commit Bot 2011-03-10 15:36:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Siddharth Mathur 2011-04-25 15:03:38 PDT
Now that Qt SDK 1.1 RC with 4.7.3 is out, the code guarded with QT_VERSION >= 0x040703 (in http://trac.webkit.org/changeset/80781) doesn't compile. 
Q_REVISION() for example, doesn't appear in qobjectdefs.h until 4.7.4. Therefore, attaching a patch which bumps up the version check requirement to 4.7.4. 

This is busting the public Symbian buildbot if we upgrade the Qt SDK to latest RC with 4.7.3.
Comment 18 Siddharth Mathur 2011-04-25 15:04:33 PDT
Created attachment 90952 [details]
fix Qt SDK 4.7.3 build bust due to Q_REVISION being undefined
Comment 19 WebKit Review Bot 2011-04-25 15:07:02 PDT
Attachment 90952 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1

Source/WebKit/qt/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 Siddharth Mathur 2011-04-25 15:07:04 PDT
Created attachment 90953 [details]
fix indent in Changelog
Comment 21 Yi Shen 2011-04-26 06:59:52 PDT
Commit Siddharth's change which is a build fix.
Comment 22 Andreas Kling 2011-04-26 08:37:03 PDT
Comment on attachment 90953 [details]
fix indent in Changelog

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

Oh this again. Huh. r=me

> Source/WebKit/qt/ChangeLog:8
> +        Macro Q_REVISION and associated qdeclrativeitem.h signals are not available in 4.7.3 headers in Nokia Qt SDK.

Typo, qdeclrativeitem.h
Comment 23 Yi Shen 2011-04-26 10:34:08 PDT
Committed r84928: <http://trac.webkit.org/changeset/84928>
Comment 24 WebKit Review Bot 2011-04-26 11:49:16 PDT
http://trac.webkit.org/changeset/84928 might have broken SnowLeopard Intel Release (WebKit2 Tests)
The following tests are not passing:
fast/loader/file-protocol-fragment.html
Comment 25 Keith Kyzivat 2011-04-28 17:39:31 PDT
This may be right for official 4.7.3 Qt builds, but trunk builds of Qt report their version as 4.8.0 -- some trunk builds (qpa) don't have the change that adds REVISION, but report their Qt versionas 4.8.0, and thus die here.

I'd like to add a check for Q_REVISION -- if it's present, all this code should work.  It's actually more accurate than checking the version number.

Attaching patch shortly.
Comment 26 Keith Kyzivat 2011-04-28 17:46:41 PDT
Created attachment 91607 [details]
Patch
Comment 27 Andreas Kling 2011-04-28 17:57:54 PDT
Comment on attachment 91607 [details]
Patch

Just checking if Q_REVISION is defined should be enough.
Comment 28 Keith Kyzivat 2011-04-28 19:32:44 PDT
Created attachment 91621 [details]
Patch
Comment 29 Csaba Osztrogonác 2011-04-29 00:43:43 PDT
(In reply to comment #27)
> (From update of attachment 91607 [details])
> Just checking if Q_REVISION is defined should be enough.

EWS won't work on closed bugs. ;) And it has Qt 4.7.2, 
because it is tha latest released Qt version now.
Comment 30 Siddharth Mathur 2011-04-29 06:11:46 PDT
Reopening
Comment 31 WebKit Commit Bot 2011-05-03 12:51:34 PDT
Comment on attachment 91621 [details]
Patch

Clearing flags on attachment: 91621

Committed r85640: <http://trac.webkit.org/changeset/85640>