[webkit-reviews] review denied: [Bug 93100] Remove minimum window size for PagePopup : [Attachment 156343] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 3 07:01:11 PDT 2012


Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 93100: Remove minimum window size for PagePopup
https://bugs.webkit.org/show_bug.cgi?id=93100

Attachment 156343: Patch
https://bugs.webkit.org/attachment.cgi?id=156343&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156343&action=review


> Source/WebCore/page/Chrome.cpp:567
> +#if ENABLE(PAGE_POPUP)
> +    if (DOMWindowPagePopup::from(window))
> +	   return FloatSize(0, 0);
> +#endif
> +    return FloatSize(100, 100);

I feel this is a violation for the concept of Supplement. Checking existence of
a Supplement in a class unrelated to the Supplement looks wrong.

We should
 - Add minimumSizeForWindow to ChromeClient.  Not Chrome.
  I think its's ok that ChromeClient.h has a default implementation which
reutrns FloatSize(100,100).
 - PagePopupChromeClient in WebPagePopupImpl.cpp overrides it.


More information about the webkit-reviews mailing list