[webkit-reviews] review granted: [Bug 106517] intrinsic min-width's don't override width for file upload controls : [Attachment 182043] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 10 09:51:59 PST 2013


Tony Chang <tony at chromium.org> has granted Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 106517: intrinsic min-width's don't override width for file upload controls
https://bugs.webkit.org/show_bug.cgi?id=106517

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=182043&action=review


> Source/WebCore/ChangeLog:3
> +	   intrinsic min-width's don't override width for file upload controls

Nit: min-width's -> min-widths

> Source/WebCore/ChangeLog:10
> +	   The preferred width is the width used in it's container's
computation

Nit: it's -> its

> Source/WebCore/ChangeLog:11
> +	   of it's intrinsic width.

Nit: it's -> its

> Source/WebCore/ChangeLog:14
> +	   the rendertree. 

Nit: rendertree -> render tree.

> Source/WebCore/rendering/RenderFileUploadControl.cpp:175
> +    RenderFileUploadControl* renderer =
const_cast<RenderFileUploadControl*>(this);
> +    float minDefaultLabelWidth = defaultWidthNumChars *
font.width(constructTextRun(renderer, font, characterAsString, style(),
TextRun::AllowTrailingExpansion));

Maybe add a FIXME about this const_cast? I guess we want to make
constructTextRun take a const renderer if possible?

> LayoutTests/ChangeLog:3
> +	   intrinsic min-width's don't override width for file upload controls

Nit: min-width's -> min-widths


More information about the webkit-reviews mailing list