[webkit-reviews] review denied: [Bug 37977] [Chromium] Font size in suggestions popup menu should be correlated with the font size of its text field. : [Attachment 54064] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 22 09:10:53 PDT 2010


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Roman
<romange at google.com>'s request for review:
Bug 37977: [Chromium] Font size in suggestions popup menu should be correlated
with the font size of its text field.
https://bugs.webkit.org/show_bug.cgi?id=37977

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 58098)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,18 @@
> +2010-04-22  Roman Gershman  <romange at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	[Chromium] Font size in suggestions popup menu should be correlated
with the font size of its text field.
> +	Currently it's not possible to test this change via LayoutTests,
> +	since test_shell does not implement suggestions popup.

The diff for the ChangeLog is touches way too much. Can you please reduce that?


>	   Unreviewed, rolling out r58069.
> Index: src/SuggestionsPopupMenuClient.cpp
> ===================================================================
> --- src/SuggestionsPopupMenuClient.cpp	(revision 58097)
> +++ src/SuggestionsPopupMenuClient.cpp	(working copy)
> @@ -156,11 +156,9 @@ void SuggestionsPopupMenuClient::initial
>      FontDescription fontDescription;
>      RenderTheme::defaultTheme()->systemFont(CSSValueWebkitControl,
>					       fontDescription);
> +    RenderStyle* style = m_textField->computedStyle();
> +   
fontDescription.setComputedSize(style->fontDescription().computedSize());

... but we can write a unit test for this, right?


More information about the webkit-reviews mailing list