[webkit-reviews] review denied: [Bug 50398] Android would like to disable rounding hacks for TextRuns in text controls : [Attachment 75401] Disable rounding hacks for TextControls

[bugzilla-daemon at webkit.org]

Steve Block <steveblock at google.com> has denied scroggo at google.com's request for
review:
Bug 50398: Android would like to disable rounding hacks for TextRuns in text
controls
https://bugs.webkit.org/show_bug.cgi?id=50398

Attachment 75401: Disable rounding hacks for TextControls
https://bugs.webkit.org/attachment.cgi?id=75401&action=review

------- Additional Comments from Steve Block <steveblock at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=75401&action=review

I saw that you uploaded a ChangeLog entry separately. You'll have to merge it
with the patch to allow this to be landed via the commit queue. Also, you need
to update the ChangeLog tests line with how this is tested, or why it can't be
tested.

> WebCore/rendering/InlineTextBox.cpp:46
> +#ifdef DISABLE_ROUNDING_HACKS

Looking at StringTruncator, which also uses TextRun::disableRoundingHacks(),
the behaviour is triggered with a method argument with a default value. Is
there a reason why we can't use the same approach here, to avoid ifdefs? Is it
a lot of work to plumb such an argument through from WebKit? I've CC'ed hyatt
and aroben who seemed to have worked on this code.

If we do use an ifdef, I think it should probably be a WTF USE macro.

> WebCore/rendering/InlineTextBox.cpp:47
> +static void disableRoundingForTextControls(const InlineTextBox* box,
TextRun& tr)

Use complete words for variable names