Bug 79186

Summary:

REGRESSION (r104060): Web font is not loaded if specified by link element dynamically inserted

Product:

WebKit

Reporter:

Yuzo Fujishima <yuzo>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, bashi, hyatt, kling, koivisto, sean

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
insert-css-link.html (test case)	none
Actual image	none
Expected image	none
patch	none
now with the test case included	kling: review+

Yuzo Fujishima

Reported 2012-02-21 19:34:07 PST

Created attachment 128113 [details] insert-css-link.html (test case) 1. Open the attached insert-css-link.html. 2. Observe both "Felipa" and "Arial" are rendered with Arial, where "Felipa" must be rendered with Felipa. This issue has been observed for: - WebKit r108205 + Safari 5.1.2 on OS X 10.6.8 - Chrome 19.0.1041.0 dev on OS X 10.6.8 This issue has *NOT* been observed for: - Safari 5.1.2 (6534.52.7) on OS X 10.6.8 - Firefox 10.0.2 on OS X 10.6.8 - Opera 11.61 on OS X 10.6.8 insert-css-link.html: <head> <style> * { font-size: 24px; } .princess { font-family: 'Felipa', 'Arial'; } </style> </head> <div class="princess">Felipa</div> <div style="font-family:Arial">Arial</div> <script> window.setTimeout( function() { var link = document.createElement("link"); link.setAttribute("href", "http://fonts.googleapis.com/css?family=Felipa"); link.setAttribute("rel", "stylesheet"); link.setAttribute("type", "text/css"); document.head.appendChild(link); }, 1); </script>

Attachments
insert-css-link.html (test case) (500 bytes, text/html) 2012-02-21 19:34 PST, Yuzo Fujishima	no flags	Details
Actual image (5.72 KB, image/png) 2012-02-21 19:38 PST, Yuzo Fujishima	no flags	Details
Expected image (5.87 KB, image/png) 2012-02-21 19:39 PST, Yuzo Fujishima	no flags	Details
patch (6.70 KB, patch) 2012-02-22 15:55 PST, Antti Koivisto	no flags	Details Formatted Diff Diff
now with the test case included (9.44 KB, patch) 2012-02-22 16:22 PST, Antti Koivisto	kling: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yuzo Fujishima

Comment 1 2012-02-21 19:35:36 PST

This can be related to http://wkb.ug/79021

Yuzo Fujishima

Comment 2 2012-02-21 19:38:51 PST

Created attachment 128115 [details] Actual image

Yuzo Fujishima

Comment 3 2012-02-21 19:39:16 PST

Created attachment 128116 [details] Expected image

Yuzo Fujishima

Comment 4 2012-02-21 21:33:56 PST

Hi Annti, Can you take a look? I confirmed that r104060 shows this issue http://trac.webkit.org/changeset/104060/ Analyze stylesheet scope to minimize style recalcs while 104052 doesn't.

Yuzo Fujishima

Comment 5 2012-02-21 21:35:41 PST

Sorry, Antti, not Annti.

Yuzo Fujishima

Comment 6 2012-02-22 01:16:53 PST

I've confirmed that applying the following patch fixes the issue. Document::analyzeStylesheetChange(...) is likely to have a bug. diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp index 5125bbb..ac2b283 100644 --- a/Source/WebCore/dom/Document.cpp +++ b/Source/WebCore/dom/Document.cpp @@ -3273,7 +3273,6 @@ void Document::analyzeStylesheetChange(StyleSelectorUpdateFlag updateFlag, const if (m_styleSheets->item(i) != newStylesheets[i]) return; } - requiresStyleSelectorReset = false; // If we are already parsing the body and so may have significant amount of elements, put some effort into trying to avoid style recalcs. if (!body() || m_hasNodesWithPlaceholderStyle)

Alexey Proskuryakov

Comment 7 2012-02-22 09:58:41 PST

<rdar://problem/10911246>

Antti Koivisto

Comment 8 2012-02-22 15:55:18 PST

Created attachment 128311 [details] patch

Ryosuke Niwa

Comment 9 2012-02-22 16:02:04 PST

Comment on attachment 128311 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=128311&action=review > Source/WebCore/ChangeLog:3 > + https://bugs.webkit.org/show_bug.cgi?id=79186 URL should show up before the summary.

Alexey Proskuryakov

Comment 10 2012-02-22 16:04:32 PST

Comment on attachment 128311 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=128311&action=review > Source/WebCore/ChangeLog:8 > + Test: fast/css/font-face-insert-link.html This doesn't seem to be included in the patch.

Antti Koivisto

Comment 11 2012-02-22 16:22:15 PST

Created attachment 128323 [details] now with the test case included

Andreas Kling

Comment 12 2012-02-22 16:33:41 PST

Comment on attachment 128323 [details] now with the test case included View in context: https://bugs.webkit.org/attachment.cgi?id=128323&action=review r=me. > LayoutTests/fast/css/font-face-insert-link.html:23 > + link.setAttribute("href", "resources/ahem.css"); If we were feeling maximally fancy, this could probably be done with a data: URL. > LayoutTests/fast/css/font-face-insert-link.html:34 > + 500 Long setTimeout() interval is long.

Antti Koivisto

Comment 13 2012-02-22 16:42:45 PST

http://trac.webkit.org/changeset/108574

Antti Koivisto

Comment 14 2012-02-22 16:43:41 PST

> Long setTimeout() interval is long. Font loading is unpredictable across platforms and I don't know of any good way to observe it.

Sean McBride

Comment 15 2012-02-23 13:12:57 PST

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

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