Bug 191834 – Extremely small monospace text size when SVG is included as an img

• Component: SVG
• Status: RESOLVED FIXED

[Derk-Jan Hartman]

When I have (monospace) Chinese characters in an SVG, they are not rendered when this SVG is included using the <img> tag, but are rendered just fine when directly included in the webpage.

Not sure if this is 'technically' correct behavior with font fallbacks and what not, but from a user's perspective, it is stupid and this difference shouldn't exist.

testcase included, incl. screenshot.

Safari 12

[Derk-Jan Hartman]

Created attachment 355272 [details]
HTML fragment

[Derk-Jan Hartman]

Created attachment 355273 [details]
original svg

[Derk-Jan Hartman]

Created attachment 355274 [details]
screenshot

[Peter Krautzberger]

Created attachment 355279 [details]
WebKitGTK+ 2.22.2 screenshot

On WebKitGTK+ 2.22.2, the text content is visible but is extremely small (i.e., unreadable).

[Simon Fraser (smfr)]

Seems to be triggered by the font-family='monospace' on the <text>. Doesn't happen if some other font is specified.

[Simon Fraser (smfr)]

In the bad case (SVG in <img>), we call fontSizeForKeyword() here:

  * frame #0: 0x0000000109530b64 WebCore`WebCore::Style::fontSizeForKeyword(keywordID=66, shouldUseFixedDefaultSize=true, document={ origin = Unique, url = , inMainFrame = 1, pageCacheState = NotInPageCache }) at StyleFontSizeFunctions.cpp:160
    frame #1: 0x000000010736b8da WebCore`WebCore::StyleBuilderCustom::applyValueFontFamily(styleResolver=0x0000000124862000, value=0x00000001248a5600) at StyleBuilderCustom.h:1019
    frame #2: 0x0000000107361ef7 WebCore`WebCore::StyleBuilder::applyProperty(property=CSSPropertyFontFamily, styleResolver=0x0000000124862000, value=0x00000001248a5600, isInitial=false, isInherit=false, registered=0x0000000000000000) at StyleBuilder.cpp:5491
    frame #3: 0x0000000107de3247 WebCore`WebCore::StyleResolver::applyProperty(this=0x0000000124862000, id=CSSPropertyFontFamily, value=0x00000001248a5600, applyState=0x00007ffeefbf0980, linkMatchMask=MatchDefault) at StyleResolver.cpp:1721
    frame #4: 0x0000000107de7f83 WebCore`WebCore::StyleResolver::CascadedProperties::Property::apply(this=0x00007ffeefbf0af8, resolver=0x0000000124862000, applyState=0x00007ffeefbf0980) at StyleResolver.cpp:2279
    frame #5: 0x0000000107de8550 WebCore`void WebCore::StyleResolver::applyCascadedPropertiesImpl<(WebCore::StyleResolver::CustomPropertyCycleTracking)1>(this=0x0000000124862000, firstProperty=2, lastProperty=29, state=0x00007ffeefbf0980) at StyleResolver.cpp:2378

and document.settings().defaultFixedFontSize() returns 0 (document.settings().defaultFontSize() is 16). So fontSizeForKeyword() returns 1, hence the tiny text.

[Simon Fraser (smfr)]

Myles, any idea why the defaultFixedFontSize default in Settings.yaml is 0, not a sensible value like 13?

[Simon Fraser (smfr)]

Created attachment 355372 [details]
Patch

[EWS Watchlist]

Comment on attachment 355372 [details]
Patch

Attachment 355372 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10093898

New failing tests:
svg/text/monospace-text-size-in-img.html

[EWS Watchlist]

Created attachment 355378 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Myles C. Maxfield]

Comment on attachment 355372 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        who omit to set it (like SVGImages) don't get broken rendering.

Do we want to make SVGImages get explicit settings, instead of just getting whatever the defaults are?

[Simon Fraser (smfr)]

(In reply to Myles C. Maxfield from comment #11)
> Comment on attachment 355372 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355372&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        who omit to set it (like SVGImages) don't get broken rendering.
> 
> Do we want to make SVGImages get explicit settings, instead of just getting
> whatever the defaults are?

Yes, I think we want this change and another change that pushes the values into SVGImage's settings.

[Simon Fraser (smfr)]

(In reply to Simon Fraser (smfr) from comment #12)
> (In reply to Myles C. Maxfield from comment #11)
> > Comment on attachment 355372 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=355372&action=review
> > 
> > > Source/WebCore/ChangeLog:9
> > > +        who omit to set it (like SVGImages) don't get broken rendering.
> > 
> > Do we want to make SVGImages get explicit settings, instead of just getting
> > whatever the defaults are?
> 
> Yes, I think we want this change and another change that pushes the values
> into SVGImage's settings.

Actually that's too hard; the SVGImage doesn't have an easy way to get to the enclosing Document. Let's go with this patch for now.

[WebKit Commit Bot]

Comment on attachment 355372 [details]
Patch

Clearing flags on attachment: 355372

Committed r238447: <https://trac.webkit.org/changeset/238447>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46214651>

[Derk-Jan Hartman]

My thanks to everyone for fixing this issue so quickly. I found this bug 3 years ago already and meant to file it several times yet never managed to get around to it. Now I feel a tad guilty for not filing it sooner ;)