Bug 130168

Summary: RenderTextControl::hasValidAvgCharWidth doesn't detect System Font
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, jonlee, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch dino: review+

Description Myles C. Maxfield 2014-03-12 17:21:32 PDT
RenderTextControl::hasValidAvgCharWidth doesn't detect System Font
Comment 1 Myles C. Maxfield 2014-03-12 17:23:01 PDT
Created attachment 226563 [details]
Patch
Comment 2 Myles C. Maxfield 2014-03-12 19:46:32 PDT
<rdar://problem/16293332>
Comment 3 Myles C. Maxfield 2014-03-12 19:55:40 PDT
Created attachment 226570 [details]
Patch
Comment 4 Dean Jackson 2014-03-13 16:29:44 PDT
Comment on attachment 226570 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226570&action=review

> Source/WebCore/ChangeLog:24
> +        * platform/graphics/Font.cpp:
> +        (WebCore::Font::hasValidAvgCharWidth):
> +        (WebCore::Font::fastAvgCharWidth):
> +        * platform/graphics/Font.h:
> +        * rendering/RenderTextControl.cpp:
> +        (WebCore::RenderTextControl::getAvgCharWidth):
> +        (WebCore::RenderTextControl::computeIntrinsicLogicalWidths):
> +        * rendering/RenderTextControl.h:
> +        * rendering/RenderTextControlMultiLine.cpp:
> +        (WebCore::RenderTextControlMultiLine::getAvgCharWidth):
> +        * rendering/RenderTextControlMultiLine.h:
> +        * rendering/RenderTextControlSingleLine.cpp:
> +        (WebCore::RenderTextControlSingleLine::getAvgCharWidth):
> +        (WebCore::RenderTextControlSingleLine::preferredContentLogicalWidth):
> +        * rendering/RenderTextControlSingleLine.h:

You should provide some per-file descriptions!

> Source/WebCore/platform/graphics/Font.cpp:501
> +        width = roundf(primaryFont()->avgCharWidth()); // FIXME: primaryFont() might not correspond to firstFamily()

. at end of comment

> Source/WebCore/platform/graphics/Font.h:157
> +    bool fastAvgCharWidth(float &width) const; // returns true on success

I wish the name was more clear. This looks like a getter, but is actually something more tricky. Maybe computeFastAvgCharWidth()? (even though there is no computation)

The other places where we do things like this seem to all be in the Audio code (mostly from a single author).

Also, let's take this opportunity to expand Avg to Average
Comment 5 Myles C. Maxfield 2014-03-15 17:18:55 PDT
http://trac.webkit.org/changeset/165686