Bug 25207 - Text not visible while external font downloading
: Text not visible while external font downloading
Status: NEW
: WebKit
Text
: 528+ (Nightly build)
: All All
: P3 Normal
Assigned To:
: http://hsivonen.iki.fi/
: InRadar
:
: 33998
  Show dependency treegraph
 
Reported: 2009-04-15 06:24 PST by
Modified: 2013-10-17 07:02 PST (History)


Attachments
Patch (8.29 KB, patch)
2010-09-13 04:42 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Patch (19.92 KB, patch)
2010-09-15 03:41 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Pre Source/WebCore move (19.97 KB, patch)
2011-01-11 02:27 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Post Source/WebCore move (20.43 KB, patch)
2011-01-11 19:17 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Patch (20.38 KB, patch)
2011-05-20 01:12 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.40 MB, application/zip)
2011-05-20 02:05 PST, WebKit Review Bot
no flags Details
Patch (28.60 KB, patch)
2011-05-23 01:18 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.43 MB, application/zip)
2011-05-23 02:16 PST, WebKit Review Bot
no flags Details
Added test for initial rendering (29.24 KB, patch)
2011-05-24 00:28 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.27 MB, application/zip)
2011-05-24 01:12 PST, WebKit Review Bot
no flags Details
Patch (31.28 KB, patch)
2011-06-06 21:24 PST, Yuzo Fujishima
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-04-15 06:24:22 PST
Happens in latest nightly (42533) on Mac and in Safari 4 beta on Windows XP.

Steps to reproduce:
1) Load http://hsivonen.iki.fi/

Actual results:
When a given run of text is to be rendered with a external font, the run of text is not drawn until the font has been downloaded.

Expected result:
If the external font is not already in cache and layout starts, expected the next best available font to be used during font download and the page to be redrawn once the external font is available.
------- Comment #1 From 2009-04-15 06:40:04 PST -------
*** Bug 25209 has been marked as a duplicate of this bug. ***
------- Comment #2 From 2009-04-15 06:40:56 PST -------
As said in my duplicate, this is the behaviour of Firefox3.1 beta 3 and Opera 10 alpha.
------- Comment #3 From 2009-04-29 22:55:02 PST -------
This does feel weird indeed, although changing the font once download completes isn't great either.
------- Comment #4 From 2009-04-29 23:09:44 PST -------
While English fonts are about 30KB, multibyte fonts are much larger and make this problem more noticeable. For example, the free Japanese M+ fonts are 350KB+ per style (http://bit.ly/mplus).
------- Comment #5 From 2009-05-06 21:04:52 PST -------
The relevant setion of CSS3 Web Fonts unfortunately suggests this behaviour (assuming ‘block on this download’ means ‘stop text layout algorithm until @font-face finishes downloading’):
4.4 “The UA may choose to block on this download or may choose to proceed to the next step while the font downloads.”
    http://www.w3.org/TR/css3-webfonts/#algorithm

I believe that this is an error in the spec, as a page in which no text initially displays appears broken to the user. This was my initial reaction to seeing Henri’s page before finding out about the bug, and I actually hit the back button before the @font-face downloaded. This sentence should be changed to “The UA must proceed to the next step while the font downloads.” I’ll post this to the www-style list.
------- Comment #6 From 2009-05-07 09:03:27 PST -------
We might consider displaying something after a timeout, but no way are we going to draw the wrong thing immediately and then flicker to the right thing.  That would look terrible.
------- Comment #7 From 2009-05-13 16:05:17 PST -------
What about prominently showing 'Loading needed fonts...' if the font download is taking more than a few seconds?
------- Comment #8 From 2009-07-08 04:43:47 PST -------
I see that both the OTF and TTF versions of the same font are downloaded. Shouldn't Webkit just settle on the Opentype (or first) version?

(Just like settling on the local() version does, right?)
------- Comment #9 From 2010-01-03 09:10:26 PST -------
I'd like to have the option to show the text or not while an external font loads as an option somewhere, something like "WebKitShowLocalFontWhileLoading 1" being an option in both the global WebKit config and whatever application is using WebKit.

Personally I would turn on showing a local font while a page loads just so the page doesn't look weird/broken. I believe Firefox 3.6b5 shows the local font until the remote is available.


Perhaps to avoid the abruptness of changing fonts we could add a text-morph transition? ;p
------- Comment #10 From 2010-08-17 01:56:21 PST -------
I think Firefox's current behaviour (showing a local font, then switching to the downloaded font once the download finishes) is preferable over just not showing any text that should be rendered with a font that is currently being downloaded.

Comment #7 (in conjunction with current behaviour) is also acceptable, although I'd show that "Downloading font..." message pretty early, maybe after half a second.
------- Comment #11 From 2010-09-12 22:24:05 PST -------
*** Bug 44404 has been marked as a duplicate of this bug. ***
------- Comment #12 From 2010-09-13 04:42:53 PST -------
Created an attachment (id=67397) [details]
Patch
------- Comment #13 From 2010-09-13 09:22:28 PST -------
(From update of attachment 67397 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=67397&action=prettypatch

> WebCore/ChangeLog:5
> +        Fix for Bug 25207 - Text not visible while extenal font downloading
Typo: extenal

> WebCore/ChangeLog:29
> +        * loader/CachedFont.cpp:
> +        (WebCore::CachedFont::beginLoadIfNeeded):
> +        * loader/CachedResourceLoader.cpp:
> +        (WebCore::CachedResourceLoader::CachedResourceLoader):
> +        (WebCore::CachedResourceLoader::startFontLoadWaitLimitTimer):
> +        (WebCore::CachedResourceLoader::fontLoadWaitLimitTimer):
> +        * loader/CachedResourceLoader.h:
> +        (WebCore::CachedResourceLoader::fontLoadWaitLimitExceeded):
> +        * platform/graphics/Font.cpp:
> +        (WebCore::Font::drawText):
> +        * platform/graphics/FontFallbackList.cpp:
> +        (WebCore::FontFallbackList::fontLoadWaitLimitExceeded):
> +        * platform/graphics/FontFallbackList.h:
> +        * platform/graphics/FontSelector.h:
There should be function-level comments about the changes.

> WebCore/platform/graphics/FontFallbackList.cpp:32
> +#include "CachedResourceLoader.h"
This is a layering violation. Platform code should not depend on higher-level WebCore code.

> WebCore/platform/graphics/FontFallbackList.cpp:145
> +    if (m_fontSelector)
> +        if (CachedResourceLoader* loader = m_fontSelector->cachedResourceLoader())
> +            if (loader->fontLoadWaitLimitExceeded())
> +                return true;
The first and second if statements require braces.

> WebCore/platform/graphics/FontSelector.h:34
> +class CachedResourceLoader;
Again, layering violation.

This patch doesnt implement the behavior described in <https://bugs.webkit.org/show_bug.cgi?id=44404#c0>, but that behavior seems preferable, especially in case the font ultimately fails to load. Has consensus been reached on www-style regarding this?
------- Comment #14 From 2010-09-15 03:41:12 PST -------
Created an attachment (id=67663) [details]
Patch
------- Comment #15 From 2010-09-15 03:49:07 PST -------
Hi, mitz,

Thank you for your review.

I've rewrote the patch to make the behavior similar to
https://bugs.webkit.org/show_bug.cgi?id=44404#c0

I've described the behavior in ChangeLog. I hope you find it reasonable.
(https://bugzilla.mozilla.org/show_bug.cgi?id=499292#c18
says "Couldn't find the www-style thread so I'm unsure of suggested timeout or
download metrics to suggest." and I don't know about the thread either.)
------- Comment #16 From 2010-10-02 17:24:05 PST -------
0.5 seconds sounds like a very short time to me. The goal is to avoid the Flash of Unstyled Text if possible, but not hide the text if the page is taking a long time to load (e.g. on mobile connection).

Mozilla is considering 3 seconds, at the moment: 
https://bugzilla.mozilla.org/show_bug.cgi?id=499292#c20

I, too, think 3-5 seconds is a reasonable timeout.

Thanks for this patch, Yuzo! I'm super excited to see this land.
------- Comment #17 From 2010-10-05 16:01:24 PST -------
Paul: can you give a link to the www-style discussion you referred to in the duplicate bug?
------- Comment #18 From 2010-10-13 19:16:21 PST -------
Not the one Simon has requested, but this is also relevant:
http://lists.w3.org/Archives/Public/www-style/2010Oct/0161.html
------- Comment #19 From 2010-10-29 00:18:35 PST -------
Ping? I can adjust the wait time before using temporary font in any way, say, to 3 sec.
------- Comment #20 From 2010-10-29 08:59:22 PST -------
There's discussion in the CSS Working Group about what the behavior should be here.
------- Comment #21 From 2010-12-03 10:38:47 PST -------
@smfr:  Any result from said discussion?
------- Comment #22 From 2010-12-03 10:43:44 PST -------
There's plenty of disagreement.
------- Comment #23 From 2010-12-27 11:12:04 PST -------
(From update of attachment 67663 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=67663&action=review

I looked over it quickly and noted a minor nit, but nothing major.

I'd like to look more closely before I r+, but before I do that, I don't understand why there isn't a test for this (r- based on no test) -- more on this in my comments.

> WebCore/ChangeLog:27
> +        No new tests because testing dynamic loading behavior is difficult to

It sounds like the behavior being fixed is something very deterministic (not a race condition, etc.).

Specifically, it occurs when the font being returned takes too long. Why can't we make the font server in the test hang while returning a result and use that to test it? (There are some xhr tests that do this.)

> WebCore/loader/CachedFont.cpp:196
> +    CachedResourceClientWalker w(m_clients);

Don't use abbreviations "w" for variable names.

> WebCore/loader/CachedFont.cpp:197
> +    while (CachedResourceClient *c = w.next())

And "c".

> WebCore/platform/graphics/FontFallbackList.cpp:93
> +        if (m_fontList[i].second) {

Consider using "continue" to avoid deep nesting.
------- Comment #24 From 2010-12-27 11:14:12 PST -------
Whoops I just noticed a lot more discussion in the bug.

It sounds like the correct behavior is being decided so perhaps it is premature for a patch?
------- Comment #25 From 2011-01-11 02:27:53 PST -------
Created an attachment (id=78504) [details]
Pre Source/WebCore move
------- Comment #26 From 2011-01-11 19:17:33 PST -------
Created an attachment (id=78642) [details]
Post Source/WebCore move
------- Comment #27 From 2011-04-26 15:43:44 PST -------
(From update of attachment 78642 [details])
This seems easy to write a test for.  I don't see why we want this though.  A 0.5 second timeout should be easy to test with a layout test.  Just point at a font which never loads (which is easy to do with an http test).
------- Comment #28 From 2011-05-20 00:33:37 PST -------
Now the specification has a section on what to render while the web font is downloading.
http://dev.w3.org/csswg/css3-fonts/#font-face-loading

Browsers are behaving as follows:

Firefox 4:
1. Text is rendered as blank for 3 seconds or so.
2. Text is rendered with fallback fonts until download completes.
3. Text is rendered with the web fonts.

Opera 11:
1. Nothing is rendered for 2 seconds or so.
2. Text is rendered with fallback fonts until download completes.
3. Text is rendered with the web fonts.

IE 9:
1. Text is rendered with fallback fonts until download completes.
2. Text is rendered with the web fonts.

Chrome, Safari:
1. Text is rendered as blank until download completes.
2. Text is rendered with the web fonts.

I think FIrefox 4's behavior is most desirable.
My patch is intended to work in basically the same way if we change the wait from 0.5 sec to 3 seconds.

Please speak up if you have objections.
------- Comment #29 From 2011-05-20 01:12:55 PST -------
Created an attachment (id=94188) [details]
Patch
------- Comment #30 From 2011-05-20 01:14:09 PST -------
I've uploaded a patch but it is not for review -- it lacks tests as of now.
------- Comment #31 From 2011-05-20 02:05:35 PST -------
(From update of attachment 94188 [details])
Attachment 94188 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8717438

New failing tests:
fast/forms/textfield-overflow.html
------- Comment #32 From 2011-05-20 02:05:42 PST -------
Created an attachment (id=94191) [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #33 From 2011-05-23 01:18:42 PST -------
Created an attachment (id=94380) [details]
Patch
------- Comment #34 From 2011-05-23 02:09:20 PST -------
Added tests.

Reviewers, can you review the patch?
------- Comment #35 From 2011-05-23 02:16:13 PST -------
(From update of attachment 94380 [details])
Attachment 94380 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8732016

New failing tests:
http/tests/webfonts/slow-loading.html
fast/forms/textfield-overflow.html
------- Comment #36 From 2011-05-23 02:16:20 PST -------
Created an attachment (id=94387) [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #37 From 2011-05-24 00:28:14 PST -------
Created an attachment (id=94579) [details]
Added test for initial rendering
------- Comment #38 From 2011-05-24 01:12:41 PST -------
(From update of attachment 94579 [details])
Attachment 94579 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8734031

New failing tests:
http/tests/webfonts/slow-loading.html
fast/forms/textfield-overflow.html
------- Comment #39 From 2011-05-24 01:12:47 PST -------
Created an attachment (id=94581) [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #40 From 2011-05-31 00:42:50 PST -------
(In reply to comment #38)
> (From update of attachment 94579 [details] [details])
> Attachment 94579 [details] [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/8734031
> 
> New failing tests:
> http/tests/webfonts/slow-loading.html

This passed locally on linux for me.


> fast/forms/textfield-overflow.html

This is probably related to https://bugs.webkit.org/show_bug.cgi?id=61768
------- Comment #41 From 2011-06-06 21:24:59 PST -------
Created an attachment (id=96194) [details]
Patch
------- Comment #42 From 2011-06-08 19:24:48 PST -------
Reviewers, ping?

Now tests passes also for cr-linux.
------- Comment #43 From 2011-06-22 23:15:50 PST -------
Ping, reviewers?

It's sad that WebKit has been lagging behind others.
(Please see comment #28.)
------- Comment #44 From 2011-08-17 18:32:26 PST -------
We're running into this problem on the Processing.js project too. We'd love to see this fixed for our users, so... pinging any reviewers.
------- Comment #45 From 2012-04-02 09:56:03 PST -------
I'd like to release the ownership.
------- Comment #46 From 2013-04-24 18:39:22 PST -------
On the Blink side, this ticket is now tracked at  http://crbug.com/235303
------- Comment #47 From 2013-10-07 11:30:23 PST -------
<rdar://problem/15167413>
------- Comment #48 From 2013-10-17 06:13:50 PST -------
We should back external font loading with aggressive speculative caching. 

- Never (within reason) remove fonts from cache
- Speculatively use cache entries even when expired
- Speculatively use cached font that is likely the same as the requested font (based on file name etc)
- Validate speculative loads asynchronously. If it turns out speculation was wrong then switch to the right font when data is available.