If you create an SVG Element and put some text in it but specify the font size to be something like 10000000000000000pt, Webkit will crash. The problem is that it's expected that at least one of the fall back fonts will always work if a font is invalid but a check for an unreasonable size is never made, thus the fall back checks keep trying valid font types with the invalid font size. Only happens on windows. The fix would be to cap the font size when it gets processed.
Created attachment 152329 [details] Applies font size capping for windows Caps font size between 0 and 1000000 when Windows processes font.
Comment on attachment 152329 [details] Applies font size capping for windows View in context: https://bugs.webkit.org/attachment.cgi?id=152329&action=review > Source/WebCore/ChangeLog:5 > + https://bugs.webkit.org/show_bug.cgi?id=91273 If there's a relevant radar you could add it here after the bugzilla link. > Source/WebCore/ChangeLog:9 > + This bug only happens on windows. The problem has to do with "This bug only happens on windows." should probably go after the explanation of the bug, and Windows is a proper noun. > Source/WebCore/ChangeLog:12 > + font sizes overflowing into negative values (which isn't being > + checked for) in the Windows specific code (Mac handles > + these values just fine). The fix is to cap the font sizes to Most of the parenthesized commentary here could probably be removed. > Source/WebCore/ChangeLog:15 > + CSS font sizes work fine in Windows because they're already capped, > + we should do the same with SVG. I know I've asked this before, but it would be nice to understand if there's a single bottleneck that all font sizes go through where we could cap them instead of two separate places. The other nice thing about the CSS bottleneck is that the capped size is exposed via the computed style to the web content, just for consistency's sake, though that likely doesn't matter with such ridiculous values in practice. > Source/WebCore/ChangeLog:17 > + No new tests (OOPS!). OOPS. Better add a test! I'll help you. > Source/WebCore/css/StyleResolver.cpp:4565 > + // between Mac and Windows Trailing period. Perhaps "to prevent changing behavior on other platforms". > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:563 > + // All Windows fonts get's processed through here (SVG and CSS). Cap the font size to be between 0 and 1000000 here. s/get's/get/ "processed" might not be the right word. You might just remove this comment entirely, the code is pretty clear. > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:564 > + float adjustedFontSize = fontDescription.computedPixelSize() > 1000000.0f || fontDescription.computedPixelSize() < 0 ? 1000000.0f : fontDescription.computedPixelSize(); s/adjusted/capped/? I'm not sure.
<rdar://problem/8355401>
I think you should look at moving all the capping into StyleBuilder.cpp's ApplyPropertyFontSize::applyValue.
Created attachment 152826 [details] Applies font size capping in StyleBuilder
Comment on attachment 152826 [details] Applies font size capping in StyleBuilder View in context: https://bugs.webkit.org/attachment.cgi?id=152826&action=review > Source/WebCore/ChangeLog:8 > + Timothy Horton <timothy_horton@apple.com> Fix this! > Source/WebCore/ChangeLog:11 > + font size in Windows, Webkit crashes. The problem has to do with WebKit has a capital K > Source/WebCore/ChangeLog:16 > + Test: svg/text/font-size-too-large-crash.svg Yay! I think you need to svn add the test or something. And the results.
Created attachment 152834 [details] Repeat of last patch (fixed changelog)
Failed to find the property value for the SVN property "svn:executable": "## -0,0 +1 ## Wharr?
Comment on attachment 152834 [details] Repeat of last patch (fixed changelog) View in context: https://bugs.webkit.org/attachment.cgi?id=152834&action=review > LayoutTests/ChangeLog:13 > + * svg/text/font-size-too-large-expected.txt: Added. > + * svg/text/font-size-too-large.svg: Added. You forgot to update these when you changed the filenames.
(In reply to comment #8) > Failed to find the property value for the SVN property "svn:executable": "## -0,0 +1 ## > > Wharr? svn propdel svn:executable svg/text/font-size-too-large.svg
Created attachment 152845 [details] Repeat of last patch (added a comment for clarification)
Created attachment 152848 [details] Repeat of last patch (I saved the file this time...)
Comment on attachment 152848 [details] Repeat of last patch (I saved the file this time...) Clearing flags on attachment: 152848 Committed r122896: <http://trac.webkit.org/changeset/122896>
All reviewed patches have been landed. Closing bug.