Bug 130165 – RenderTextControl does not need OS X-specific behavior

• Status: REOPENED
• Component: Layout and Rendering

[Myles C. Maxfield]

RenderTextControl inspects a family name to see if it starts with a '.' This is OSX-specific.

[Myles C. Maxfield]

Created attachment 227085 [details]
Patch

[Dean Jackson]

Comment on attachment 227085 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        RenderTextControl switches on OSX behavior

Please change bug title to "RenderTextControl does not need iOS-specific behavior"

> Source/WebCore/ChangeLog:18
> +        * platform/graphics/Font.cpp:
> +        (WebCore::Font::hasValidAverageCharWidth):
> +        (WebCore::Font::fastAverageCharWidthIfAvailable):

You should describe what you did here, because it isn't removing platform-specific code, or at least doesn't look like it is.

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:-354
> -    // For text inputs, IE adds some extra width.
> -    if (maxCharWidth > 0.f)
> -        result += maxCharWidth - charWidth;

How do we know we can get rid of this too?

[Dean Jackson]

Comment on attachment 227085 [details]
Patch

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

>> Source/WebCore/ChangeLog:3
>> +        RenderTextControl switches on OSX behavior
> 
> Please change bug title to "RenderTextControl does not need iOS-specific behavior"

I meant OS X specific, of course :)

[Myles C. Maxfield]

That code is guarded behind a check which is never true. Therefore, it wasn't getting hit. By unifying the two code paths, I was changing the behavior of the guard, which was making this code be (erroneously) hit, so I needed to take it out.

[Build Bot]

Created attachment 227095 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Created attachment 227096 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Myles C. Maxfield]

Looks like the OS/2 table has positive values for xAvgCharWidth, rather than negative values which I had anticipated. This patch is bad.

[Myles C. Maxfield]

Reopening to attach new patch.

[Myles C. Maxfield]

Created attachment 227341 [details]
WIP

[Ahmad Saleem]

(In reply to Myles C. Maxfield from comment #9)
> Created attachment 227341 [details]
> WIP

@myles - We do have this code still:

https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/FontCascade.cpp#L354

Is this patch needed anymore? Thanks!