[webkit-reviews] review denied: [Bug 77843] [BlackBerry] Upstream ChromeClientBlackBerry.{h, cpp} : [Attachment 125767] Patch v2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 7 07:25:14 PST 2012
Rob Buis <rwlbuis at gmail.com> has denied review:
Bug 77843: [BlackBerry] Upstream ChromeClientBlackBerry.{h, cpp}
https://bugs.webkit.org/show_bug.cgi?id=77843
Attachment 125767: Patch v2
https://bugs.webkit.org/attachment.cgi?id=125767&action=review
------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=125767&action=review
r- since I think it can be cleaned up a lot.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:89
> + m_webPagePrivate->m_dumpRenderTree->addMessageToConsole(message,
lineNumber, sourceID);
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:108
> + return
m_webPagePrivate->m_dumpRenderTree->runJavaScriptConfirm(message);
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:113
> +
Why empty line?
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:118
> + WebString clientResult;
This can be removed to where it is actually used.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:123
> + }
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:142
> + return; // The window dimensions are fixed in the RIM port.
Why return; ?
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:159
> + return 1.0f;
Should be just 1.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:279
> + }
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:311
> + m_webPagePrivate->m_dumpRenderTree->windowCreated(webPage);
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:411
> + return
m_webPagePrivate->m_dumpRenderTree->runBeforeUnloadConfirmPanel(message);
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:426
> + m_webPagePrivate->m_dumpRenderTree->setStatusText(status);
We need #if ENABLE_DRT here.
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:539
> +void ChromeClientBlackBerry::runOpenPanel(WebCore::Frame*,
PassRefPtr<WebCore::FileChooser> chooser)
Do we really need WebCore:: prefix everywhere?
> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:607
> + m_webPagePrivate->m_backingStore->d->repaint(updateRect, true
/*contentChanged*/, true /*immediate*/);
Could call invalidateContentsAndWindow to indicate what we are actually doing.
More information about the webkit-reviews
mailing list