Bug 96648 - Replace qInstallMsgHandler() with qInstallMessageHandler()
Summary: Replace qInstallMsgHandler() with qInstallMessageHandler()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-13 07:32 PDT by Kai Koehne
Modified: 2012-09-28 03:34 PDT (History)
9 users (show)

See Also:


Attachments
patch using the new API (6.34 KB, patch)
2012-09-13 07:37 PDT, Kai Koehne
no flags Details | Formatted Diff | Diff
patch using the new API (6.35 KB, patch)
2012-09-25 04:12 PDT, Kai Koehne
no flags Details | Formatted Diff | Diff
patch using the new API (4.91 KB, patch)
2012-09-28 02:12 PDT, Kai Koehne
no flags Details | Formatted Diff | Diff
patch using the new API (6.43 KB, patch)
2012-09-28 02:30 PDT, Kai Koehne
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Koehne 2012-09-13 07:32:04 PDT
In qt5 / qtbase, qInstallMsgHandler() got deprecated, and replaced by qInstallMessageHandler. QtWebkit should use the new API.
Comment 1 Kai Koehne 2012-09-13 07:37:33 PDT
Created attachment 163869 [details]
patch using the new API
Comment 2 Peter Gal 2012-09-25 02:54:03 PDT
(In reply to comment #1)
> Created an attachment (id=163869) [details]
> patch using the new API

Just a sidenote, I've found this about qPrintable:
http://lists.trolltech.com/qt-interest/2007-12/thread00508-0.html

Is this still valid? if so we could use .constData() as in:
http://qt-project.org/doc/qt-5.0/qtglobal.html#qInstallMessageHandler
Comment 3 Kai Koehne 2012-09-25 03:00:30 PDT
qPrintable(string) 

is actually just a macro that expands to

QString(string).toLocal8Bit().constData() 

The e-mail of qt-interest was about whether the temporary QByteArray created by toLocal8Bit() is guaranteed to be still valid by the time printf(...) is executed, and the answer (at the bottom of the thread) is that it is: It can only be destroyed after the whole statement has executed. So

printf("%s", qPrintable(x));

is okay while

const char * str = qPrintable(x);
printf("%s", str);

isn't.
Comment 4 Jocelyn Turcotte 2012-09-25 03:03:00 PDT
Comment on attachment 163869 [details]
patch using the new API

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

> Source/WebKit2/ChangeLog:4
> +	https://bugs.webkit.org/show_bug.cgi?id=96648

Spaces instead of tabs please.
Comment 5 Kai Koehne 2012-09-25 04:12:31 PDT
Created attachment 165577 [details]
patch using the new API

fixed tab usage.
Comment 6 Andras Becsi 2012-09-28 02:04:52 PDT
Comment on attachment 165577 [details]
patch using the new API

As the purple bubbles indicate your patch does not apply. Could you update it to WebKit trunk so that the early warning system and the commit queue can process it.
Comment 7 Kai Koehne 2012-09-28 02:12:58 PDT
Created attachment 166179 [details]
patch using the new API

Updated to apply to webkit / master.
Comment 8 Andras Becsi 2012-09-28 02:20:10 PDT
Comment on attachment 166179 [details]
patch using the new API

This patch does not have a changelog.
Comment 9 Kai Koehne 2012-09-28 02:30:23 PDT
Created attachment 166184 [details]
patch using the new API

Duh, I'm sorry. I hope I got it this time right...
Comment 10 Andras Becsi 2012-09-28 02:39:17 PDT
Comment on attachment 166184 [details]
patch using the new API

LGTM. Simon or Jocelyn can toggle the review flag for you :)
Comment 11 Jocelyn Turcotte 2012-09-28 03:30:34 PDT
Comment on attachment 166184 [details]
patch using the new API

Thanks a lot :)
Comment 12 WebKit Review Bot 2012-09-28 03:34:55 PDT
Comment on attachment 166184 [details]
patch using the new API

Clearing flags on attachment: 166184

Committed r129870: <http://trac.webkit.org/changeset/129870>
Comment 13 WebKit Review Bot 2012-09-28 03:34:58 PDT
All reviewed patches have been landed.  Closing bug.