Bug 130168 - RenderTextControl::hasValidAvgCharWidth doesn't detect System Font
Summary: RenderTextControl::hasValidAvgCharWidth doesn't detect System Font
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-12 17:21 PDT by Myles C. Maxfield
Modified: 2014-03-15 17:18 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.74 KB, patch)
2014-03-12 17:23 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (13.85 KB, patch)
2014-03-12 19:55 PDT, Myles C. Maxfield 		dino: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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