[webkit-reviews] review denied: [Bug 85907] [BlackBerry] Enable PAGE_POPUP in make file, and implement required methods : [Attachment 140947] Fix style issue and add more details in Changelog.

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Crystal Zhang <haizhang at rim.com>'s
request for review:
Bug 85907: [BlackBerry] Enable PAGE_POPUP in make file, and implement required
methods
https://bugs.webkit.org/show_bug.cgi?id=85907

Attachment 140947: Fix style issue and add more details in Changelog.
https://bugs.webkit.org/attachment.cgi?id=140947&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=140947&action=review


Looks good, can be improved some more.

> Source/WebKit/blackberry/Api/WebPage.cpp:378
> +

You don't need the empty line.

> Source/WebKit/blackberry/Api/WebPage.cpp:6106
> +    return (d->m_selectPopup) ? false : true;

It does not need to be this explicit, just say return d->m_selectPopup;

> Source/WebKit/blackberry/Api/WebPage.h:341
> +    WebCore::PagePopupBlackBerry* popup();

Can some be const, like hasOpenedPopup?

> Source/WebKit/blackberry/Api/WebPage_p.h:544
> +

No need for empty line.

> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:314
> +    }

You can move the webPopup->sendCreatePopupWebViewRequest(); usage to here so
you state it once.