Summary: | RenderTextControl does not need OS X-specific behavior | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||||
Component: | Layout and Rendering | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||||
Status: | REOPENED --- | ||||||||||||
Severity: | Normal | CC: | ahmad.saleem792, dino, jonlee | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Myles C. Maxfield
2014-03-12 16:26:45 PDT
Created attachment 227085 [details]
Patch
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? 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 :) 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. 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
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
Looks like the OS/2 table has positive values for xAvgCharWidth, rather than negative values which I had anticipated. This patch is bad. Reopening to attach new patch. Created attachment 227341 [details]
WIP
(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! |