[Webkit-unassigned] [Bug 63582] [Qt] REGRESSION(r87797): Broke KDEWebKit's custom QNAM.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63582


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #99001|review?                     |review-
               Flag|                            |




--- Comment #4 from Andreas Kling <kling at webkit.org>  2011-06-29 08:27:36 PST ---
(From update of attachment 99001)
View in context: https://bugs.webkit.org/attachment.cgi?id=99001&action=review

> Source/WebKit/qt/ChangeLog:5
> +        [Qt] Regression: Fix for 49650 breaks custom QNAM implementations...

Please end your sentences with one period, not three.

> Source/WebKit/qt/ChangeLog:8
> +        This is an improvement of the regression patch committed to address bug 49650.

This adds no information.

> Source/WebKit/qt/ChangeLog:9
> +        It allows for immediate emition of the QWebPage::unsupportedContent signal

emition -> emission

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:215
> +    qRegisterMetaType<QNetworkReply*>("QNetworkRepyPtr");

Typo, should be "QNetworkReplyPtr".
Is this the right place to register meta types?
Note that this code may execute more than once.

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:242
> +    // NOTE: This should be a signal-to-signal connection were it not for bugs.

should -> would

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1699
> +    if (page->property("SKIP_QTBUG_18718_WORKAROUND").toBool()

I don't like the name of this property. Can we make it something that describes what it does instead?
_q_emitUnsupportedContentDirectly perhaps?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.