1118981 - font rendering does not always respect fallback delay time

Status: VERIFIED FIXED

(Core :: Graphics: Text, defect, P2)

Description

[Yaron]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150106233618

Steps to reproduce:

Go to:
http://www.roboform.com/download/news-windows


Actual results:

With FF 35:

Initially, the site is displayed in a certain font and after a brief moment the page is re-rendered and a different font is used.

Also, many sites in Hebrew are displayed in new different fonts.
It seems that some options are missing in "Options -> Content -> Alternative Encoding".

Comment 1

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I am able to reproduce this issue on Windows 7 x64 using Firefox 35 RC (build2), the latest Firefox 36 Aurora, and the latest Firefox 37 Nightly, using the URL provided by Yaron.

The issue is a regression:
- last good - 2014-09-25
- first bad - 2014-09-26
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1735ff2bb23e&tochange=818f353b7aa6

Comment 2

[John Daggett (:jtd)]

Possible regression from bug 998869, which landed within the pushlog range listed.

> Also, many sites in Hebrew are displayed in new different fonts.

Hmmm, that's curious. Do have a few URL's for sites where you see the change?

Comment 3

[Yaron]

http://www.news1.co.il/

This is prior to 35:
http://s14.postimg.org/syb5qgme9/Old.png

And this is with 35:
http://s11.postimg.org/66mtvnu6r/FF35.png

It happens in many other sites as well.

Comment 4

[Yaron]

I don't know if this is relevant:
"Windows-1255" does not appear in "Fallback Character Encoding".

http://s13.postimg.org/eyh711srb/FF35.png
http://s15.postimg.org/l7skk7d63/Old.png

Comment 5

[John Daggett (:jtd)]

(In reply to Yaron from comment #3)
> http://www.news1.co.il/
> 
> This is prior to 35:
> http://s14.postimg.org/syb5qgme9/Old.png
> 
> And this is with 35:
> http://s11.postimg.org/66mtvnu6r/FF35.png
> 
> It happens in many other sites as well.

Thanks, Yaron. Could I ask you to log a separate bug about the Hebrew fallback fonts? My guess is the same set of changes is the cause but it's really a separate issue from the downloadable fonts flashing problem here.

Comment 6

[John Daggett (:jtd)]

Steps to reproduce:

Create a new test profile and set 'gfx.downloadable_fonts.fallback_delay' to 20000 in about:config.

1. Start Firefox with the test profile
2. Clear the browser cache
3. Go to http://www.roboform.com/download/news-windows

Result:

Headings such as "Version 7.9.11.5 -- Dec 02, 2014" will flash as the font load completes.

Expected result:

Given that the fallback delay is 20 seconds, elements using downloadable fonts should display as blank text until the fonts are fully loading. No flashing should occur.

Comment 7

[Yaron]

Thank you John. I appreciate your work.

Bug 1119619.

Comment 8

[John Daggett (:jtd)]

Attachment: screenshot, before and after font load completes

With fallback delay set to 20secs most of the text on the left shouldn't be displayed.

Comment 9

[John Daggett (:jtd)]

So the problem is that the mSkipDrawing member var is not set when constructing the fontlist. Since the fontlist may include userfonts that are *already* loading, this will lead to the fallback font showing before the load delay time.

Comment 10

[John Daggett (:jtd)]

Attachment: patch, initialize mSkipDrawing for already loading fonts

Check the load state of userfonts when adding to the fontlist. Set mSkipDrawing as appropriate.

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 8548710 [details] [diff] [review]
patch, initialize mSkipDrawing for already loading fonts

Review of attachment 8548710 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, that makes sense.

Comment 12

[Yaron]

Thank you John and Jonathan.
What about Bug 1119619? It's a major issue in many pages.
See also Bug 604991.

@Simon,
Can you have a look please?

Comment 13

[John Daggett (:jtd)]

Pushed to inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ed011257a25

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7ed011257a25

Comment 15

[John Daggett (:jtd)]

Comment on attachment 8548710 [details] [diff] [review]
patch, initialize mSkipDrawing for already loading fonts

Approval Request Comment
[Feature/regressing bug #]: Bug 998869 improved lazy loading of downloable fonts but in some instances caused the fallback font display timeout to be ignored, resulting in flashes of fallback font text.
[User impact if declined]: flashes of fallback font text in some cases (e.g. the camera app in FxOS, bug 1097233)
[Describe test coverage new/current, TBPL]: all downloadable font reftests pass
[Risks and why]: change is minor and simply initializes the state of the fontlist more accurately
[String/UUID change made/needed]: N/A

Comment 16

[John Daggett (:jtd)]

Pushed to aurora/beta:
https://hg.mozilla.org/releases/mozilla-aurora/rev/31cfd9095d71
https://hg.mozilla.org/releases/mozilla-beta/rev/beb62e1ad523

Comment 18

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced the initial issue on Firefox 35 RC, build ID: 20150106233618.

Confirming this fix on Windows 7 64-bit, Ubuntu 14.04 64-bit and Mac OS X 10.9.5 using:
1. Latest Nightly, build ID: 20150121030203;
2. Latest Aurora, build ID: 20150121004011;
3. Firefox 36 beta 2, build ID: 20150120155007.