Bug 68090 – SVG as image uses very tiny default font-size

• Component: SVG
• Status: RESOLVED FIXED

[Daniel Holbert]

Created attachment 107354 [details]
testcase

STR: Load attached testcase

EXPECTED RESULTS: Text is same size (and readible) in both images.
ACTUAL RESULTS: Text is unreadable in the top image.

The only difference between the two images is that I explicitly specify font-size='16' in the lower one.
In Firefox & Opera, this is the default size, so it doesn't make a difference in the rendering.  In Chromium 14, though, the default size seems to be teeny tiny and unreadable.  This is <img>-specific -- if you edit the testcase to use <embed> or <iframe> instead of <img>, then the font becomes reasonably-sized.

Chromium version: 14.0.835.126 (Developer Build 99097 Linux) Ubuntu 11.10

[Florin Malita]

Created attachment 119294 [details]
Patch

[Florin Malita]

The problem is that SVGImage creates an embedded Page and indirectly a fresh/default-initialized Settings object. Hence, defaultFontSize is 0 for embedded text elements.

The patch above is a fist attempt at addressing the issue by inheriting relevant values from the current settings. There're a couple of things I'm unsure about though:

1) The plumbing for making the current Settings reference available in SVGImage::dataChanged() seems awkward, if even safe. Is there a better way to gain access to the current settings in that method?

2) I couldn't find a way to implement the test programmatically and ended up with a text pixel test - which I presume will have some variance across platforms. Is this avoidable?

Cached resources may be shared between documents that have different settings. It seems very wrong to arbitrarily let the first document requesting the image to be the one determining its default font size.

See also bug 20846.

[Nikolas Zimmermann]

Comment on attachment 119294 [details]
Patch

This test should move to LayoutTests/svg/as-image, I think its more suitable there, as it's really SVGImage-specific.
I'm not too familiar with the design of Settings & co, and thus would like to hear another opinion, before touching r? state.
Great that you discovered the root of this problem!

[Nikolas Zimmermann]

(In reply to comment #3)
> Cached resources may be shared between documents that have different settings. It seems very wrong to arbitrarily let the first document requesting the image to be the one determining its default font size.

That's true. But the common case is that a resources is shared between documents with the same settings (in eg. a regular browsing session).

Fixing that properly would involve marking CachedResources as "unique", to avoid to share it with another document. I worked on this during the early SVGImage-size-negotiation patches, and even had a patch for this. Though it's complex to get it right. I only implemented a simple logic, you could mark one resource as unique, and then the next time that resource was requested, it had to be reloaded from the wire. This is bad.

A correct design would allow us to, check whether a CachedResource should be unique or not, but at the point when the same resource is requested a 2nd time. Using the type of resources, say we know it's a SVGImage, we can then determine whether its okay to share these resources, or not.
That still wouldn't handle dynamic changes, eg. by changing the default font size from Safari. That would involve checking all resources, eg:

If two documents a.html / b.html share foo.svg, with the same min. font-size of 16, and you'd change a.htmls font-size to 14, then a new CachedResource would need to appear, that corresponds to a SVGImage with the different Settings object. (It's not necessary to actually create two CachedResources, one could invent another cache of SVGImage-by-Settings, inside of it, similar to how its done with SVGImageCache).

Anyhow, I'm just brainstorming, and wanted to note that the "real issue" is hard to fix.
This "workaround" will catch most common cases though, and fixes a real problem, that now hits us badly.

It's not obvious to my that *any* document's settings should affect the rendering of SVG images, nor that the settings that apply to them should be customizable. A global, hard-wired Settings object (say, initialized with WebKit's default defaults) could provide settings for all SVG images.

[Florin Malita]

(In reply to comments #3, #7)
> Cached resources may be shared between documents that have different settings. 

I see. Is this a concern in practice through? I'm fairly new to WK and not familiar with the area - can you describe what it would take to end up in a situation where resource-sharing documents have different settings?


> It seems very wrong to arbitrarily let the first document requesting the image to be the one determining its default font size.

Yes - in light of the above it does. But, pragmatically speaking, I'm with Nikolas: currently all cases are broken; if this workaround fixes the common use case, then it may be an acceptable temporary band-aid (for 20846 too) while a proper solution is implemented.


> It's not obvious to my that *any* document's settings should affect the rendering of SVG images, nor that the settings that apply to them should be customizable.

I agree it's not made obvious in the spec, but section 6.1 does reference CSS2 for the definition of shared attributes (IIRC that covers attribute resolution all the way to UA settings). And it seems the right thing to do (keeping HTML & SVG text elements consistent).

I've verified the behavior in FF & Opera: they both honor user font settings when rendering image-embedded SVGs. Furthermore, updated settings are reflected immediately.

[Alexey Proskuryakov]

I agree with Mitz - even an image SVG is still a separate document, and having embedding documents affect embedded ones is generally not advisable.

> A global, hard-wired Settings object (say, initialized with WebKit's default defaults) could provide settings for all SVG images.

Indeed.

[Eric Seidel (no email)]

Comment on attachment 119294 [details]
Patch

Really?  This seems strange.  SVG when used as an <img> should just be like a .gif used as an img.. We don't change the font-size (well, can't) for gif. :)  pdf is another good example.. even though Chromium doesn't support pdf-as-image (but Safari did... and may still?)

[Eric Seidel (no email)]

If authors want this behavior, tey should just embed the <svg> code directly in the HTML.  That would use CSS interitance and HTML5 parsing rules.  It's not clear that SVG-as-image makes sense for the web platform in general.

[Eric Seidel (no email)]

(full disclosure, I wrote WebKit's SVGImage support, and associated hacks.)

[Nikolas Zimmermann]

(In reply to comment #9)
> 
> I've verified the behavior in FF & Opera: they both honor user font settings when rendering image-embedded SVGs. Furthermore, updated settings are reflected immediately.

Alexey/Dan:
The SVG intrinsic size is already dependent on the host document - as explicitly specified in SVG and CSS 2.1. I fully agree, that its awkward that the font-size of an embedded SVG is dependent on the host document, but this is how its implicitly specified, and how it works in Opera/FF.

Of course, it's much easier for us, if we'd hard-code the default settings for the embedded SVG - I'm also not aware of any real world issue this would introduce, except compatibility.

If we want to ignore this specifically, we should get folks from Opera/FF on board, and fix it in all our implementations.

I'd propose we ignore the dynamic part of the font-size changes for now, and hardcode the default settings for SVGImage, for now.

[Alexey Proskuryakov]

I'm not sure if we're on the same page here.

Hardcoding settings doesn't seem to be an option anyone suggests. What I'm thinking of is using browser defaults for font size, not whatever size embedding document's settings object has. In practice, per-document settings normally have the same sizes as browser defaults say.

[Tim Horton]

(In reply to comment #14)
> I'm not sure if we're on the same page here.
> 
> Hardcoding settings doesn't seem to be an option anyone suggests. What I'm thinking of is using browser defaults for font size, not whatever size embedding document's settings object has. In practice, per-document settings normally have the same sizes as browser defaults say.

Using the browser defaults, as Dan and Alexey suggest, is exactly what happens in Firefox and Opera. SVG-in-<img> does *not* inherit text size from the parent element, it uses the default font-size that a fresh document would have. Inheriting there would make no sense, and would not match any browser.

My vote is definitely for using the defaults, if only for consistency's sake.

[Nikolas Zimmermann]

(In reply to comment #14)
> I'm not sure if we're on the same page here.
Oh we are I meant to say "hard-wired", not "hard-coded", agreeing to Dans comment "A global, hard-wired Settings object (say, initialized with WebKit's default defaults) could provide settings for all SVG images."
> 
> Hardcoding settings doesn't seem to be an option anyone suggests. What I'm thinking of is using browser defaults for font size, not whatever size embedding document's settings object has. In practice, per-document settings normally have the same sizes as browser defaults say.
Right.

[Alexey Proskuryakov]

See also: bug 76162.

[Alexey Proskuryakov]

*** Bug 93770 has been marked as a duplicate of this bug. ***

[Florin Malita]

*** Bug 103691 has been marked as a duplicate of this bug. ***

[Philip Rogers]

*** Bug 125788 has been marked as a duplicate of this bug. ***

[Tamas Gergely]

(In reply to comment #20)
> *** Bug 125788 has been marked as a duplicate of this bug. ***

Is it right to assume that the default settings (default font size, default encoding, etc.) are absolutely global in a browser, and cannot be set as per page, tab or window? If yes, then changing these attributes to global (static in the class Settings) could solve the problem, however raises another one: on changing the defaults (from anywhere), all docs should be notified and rendered again. I'm not sure whether is should be an automatic, or should be initiated by the one who changed the defaults.

[Alexey Proskuryakov]

> Is it right to assume that the default settings (default font size, default encoding, etc.) are absolutely global in a browser, and cannot be set as per page, tab or window?

No, settings are not global. For example, when a web view is used for part of browser UI, it's not affected by user specified settings for web content.

<rdar://problem/12563956>

[Alexey Proskuryakov]

*** Bug 52892 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

Created attachment 253236 [details]
Patch

[Said Abou-Hallawa]

Created attachment 253242 [details]
Patch

[Said Abou-Hallawa]

This is the summery of the solutions which were discussed to fix this bug:

1) Tie the page which is created for the non-interactive SVG with the preferences so when they change, the resource page gets laid out:
  -- no infrastructure exists to make calls from WebCore to WebKit
  ++ being compatible with FireFox
2) Inherit the global settings from the top-level document:
  ++ The patch which Florin Malita uploaded does exactly that.
  ++ If preferences do not change while the SVG is in the cache, the SVG text with default font size will be rendered the same as the other text.
  -- The layout of the SVG will depend on when it is cached. Once it is cached it will not be notified if the preferences change.
  -- Different from FireFox if the preferences change.
3) Have a separate preference key for the non-interactive SVG default font size
  -- Safari removed its default font size from the preferences dialog so it does not make sense to add a new one just for the non-interactive SVG default font size.
  -- Different from FireFox.
4) After creating the page for the non-interactive resource, explicitly set the defaultFontSize to 16
  ++ The SVG text with default font size will be rendered the same regardless of the preferences when the SVG is firstly loaded.
  -- All the text with default font size in the page will change if the preferences change but the text in the non-interactive SVG will not.
  -- Different from FireFox.
5) Change initial value of Settings.m_defaultFontSize to be 16 instead of zero
  ++ Same as (4) but a little bit cleaner.

I uploaded two patches for solutions (4) and (5).

[Build Bot]

Comment on attachment 253242 [details]
Patch

Attachment 253242 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5980583440154624

New failing tests:
svg/text/text-default-font-size.html

[Build Bot]

Created attachment 253249 [details]
Archive of layout-test-results from ews103 for mac-mavericks

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

[Darin Adler]

Comment on attachment 253242 [details]
Patch

This change looks fine. r=me

I also think we should be doing this for a lot more settings, not just defaultFontSize. For example, defaultFixedFontSize. Basically the default defaults in Settings should match the default defaults in WebPreferences unless there is a good reason they should *not* match. Someone should follow up and do that because I think we could have similar issues with various other settings too.

I also think that in the future SVG image rendering might want to respect *some* settings from the host page. Not sure which ones at this time, though.

[Said Abou-Hallawa]

Created attachment 253528 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 253528 [details]
Patch

Clearing flags on attachment: 253528

Committed r184719: <http://trac.webkit.org/changeset/184719>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

*** Bug 100269 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

*** Bug 100269 has been marked as a duplicate of this bug. ***