Bug 36529

Created attachment 51494 [details] add #ifndef QT_NO_TEXTCODEC ~ #endif

Created attachment 51495 [details] add #ifndef QT_NO_ACTION ~ #endif

Comment on attachment 51494 [details] add #ifndef QT_NO_TEXTCODEC ~ #endif How does this even compile? What encodings does it use if not the TextCodecQt?

Comment on attachment 51495 [details] add #ifndef QT_NO_ACTION ~ #endif What does this do, and why is it useful? Please explain in the ChangeLog.

(In reply to comment #3) > (From update of attachment 51494 [details]) > How does this even compile? What encodings does it use if not the TextCodecQt? TextCodecQt is useless if QTextCodec is disabled in Qt. No codec would be used.

Created attachment 51640 [details] add #ifndef QT_NO_ACTION ~ #endif to use QAction made a smaller patch and added description in the ChangeLog.

Comment on attachment 51494 [details] add #ifndef QT_NO_TEXTCODEC ~ #endif The patch itself is fine, but I don't think we should add these defines, as WebKit cannot be used without codecs. Therefore the right thing to do is to disable the compilation of WebKit altogether if Qt is compiled with QT_NO_TEXTCODEC.

(In reply to comment #6) > Created an attachment (id=51640) [details] > add #ifndef QT_NO_ACTION ~ #endif to use QAction > > made a smaller patch and added description in the ChangeLog. I'm wondering about this one, too. The API is rendered almost useless if it's not possible to access the actions. There isn't much you can do with WebKit if you don't have QAction objects. Tasuku, what's the use-case for this? Shouldn't WebKit be disabled in this configuration instead?

(In reply to comment #7) > (From update of attachment 51494 [details]) > The patch itself is fine, but I don't think we should add these defines, as > WebKit cannot be used without codecs. Therefore the right thing to do is to > disable the compilation of WebKit altogether if Qt is compiled with > QT_NO_TEXTCODEC. (In reply to comment #8) > (In reply to comment #6) > I'm wondering about this one, too. The API is rendered almost useless if it's > not possible to access the actions. There isn't much you can do with WebKit if > you don't have QAction objects. > > Tasuku, what's the use-case for this? Shouldn't WebKit be disabled in this > configuration instead? From a general browser point of view, WebKit is useless without these two features. On the other hand, for people who uses WebKit as a platform for html and javascript apps, it can be useful even if such features are not available. WebKit should be enabled as much as possible.

(In reply to comment #9) > (In reply to comment #7) > > (From update of attachment 51494 [details] [details]) > > The patch itself is fine, but I don't think we should add these defines, as > > WebKit cannot be used without codecs. Therefore the right thing to do is to > > disable the compilation of WebKit altogether if Qt is compiled with > > QT_NO_TEXTCODEC. > > (In reply to comment #8) > > (In reply to comment #6) > > I'm wondering about this one, too. The API is rendered almost useless if it's > > not possible to access the actions. There isn't much you can do with WebKit if > > you don't have QAction objects. > > > > Tasuku, what's the use-case for this? Shouldn't WebKit be disabled in this > > configuration instead? > > From a general browser point of view, WebKit is useless without these two > features. On the other hand, for people who uses WebKit as a platform for html > and javascript apps, it can be useful even if such features are not available. > WebKit should be enabled as much as possible. Even if you just want to display html and run JavaScript you're going to need text codecs, to properly convert the 8-bit input into unicode.

(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #7) > > > (From update of attachment 51494 [details] [details] [details]) > > > The patch itself is fine, but I don't think we should add these defines, as > > > WebKit cannot be used without codecs. Therefore the right thing to do is to > > > disable the compilation of WebKit altogether if Qt is compiled with > > > QT_NO_TEXTCODEC. > > > > (In reply to comment #8) > > > (In reply to comment #6) > > > I'm wondering about this one, too. The API is rendered almost useless if it's > > > not possible to access the actions. There isn't much you can do with WebKit if > > > you don't have QAction objects. > > > > > > Tasuku, what's the use-case for this? Shouldn't WebKit be disabled in this > > > configuration instead? > > > > From a general browser point of view, WebKit is useless without these two > > features. On the other hand, for people who uses WebKit as a platform for html > > and javascript apps, it can be useful even if such features are not available. > > WebKit should be enabled as much as possible. > > Even if you just want to display html and run JavaScript you're going to need > text codecs, to properly convert the 8-bit input into unicode. I see. I'll find a way to disable WebKit when Qt is compiled without textcodec support. How about QAction?

(In reply to comment #11) > > Even if you just want to display html and run JavaScript you're going to need > > text codecs, to properly convert the 8-bit input into unicode. > > I see. I'll find a way to disable WebKit when Qt is compiled without textcodec > support. How about QAction? I'm inclined to apply the same argument to QAction. There's very basic stuff that can't be done without QActions, such as determining whether you can go back or forward in the history, or creating a context menu. I propose to discuss this on the mailing list first, I'd like to know if there are other people interested in having this feature. I've started a thread on the mailing list: https://lists.webkit.org/pipermail/webkit-qt/2010-April/000332.html Let's find a concensus there first and then continue with these patches.

Comment on attachment 51640 [details] add #ifndef QT_NO_ACTION ~ #endif to use QAction Clearning review as per comment 12.

Comment on attachment 51640 [details] add #ifndef QT_NO_ACTION ~ #endif to use QAction Clearing flags on attachment: 51640 Committed r57327: <http://trac.webkit.org/changeset/57327>

All reviewed patches have been landed. Closing bug.

http://trac.webkit.org/changeset/57327 might have broken Leopard Intel Release (Tests) The following changes are on the blame list: http://trac.webkit.org/changeset/57326 http://trac.webkit.org/changeset/57327

Revision r57327 cherry-picked into qtwebkit-2.0 with commit 2ad5e21e9a363de2b2c837a04d37e63460d4df59