[webkit-reviews] review granted: [Bug 49868] REGRESSION (r71934): Main search field at Apple tech specs does not accept typing : [Attachment 74812] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 49868: REGRESSION (r71934): Main search field at Apple tech specs does not
accept typing
https://bugs.webkit.org/show_bug.cgi?id=49868

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=74812&action=review

I wonder if some of the code in RenderBlock::hasLineIfEmpty can now be deleted.


> LayoutTests/fast/forms/disabled-search-input.html:59
>  \ No newline at end of file

We normally put those end-of-file newlines in WebKit source files.

> WebCore/rendering/TextControlInnerElements.cpp:54
> +    virtual bool hasLineIfEmpty() const { return true; }

I slightly prefer that such overrides be private rather than public. It’s
typically a programming error if someone calls the function on a
RenderTextControlInnerBlock* directly, and it’s nice to have this caught at
compile time. And the privacy has no effect on overriding.