Bug 79186 - REGRESSION (r104060): Web font is not loaded if specified by link element dynamically inserted
Summary: REGRESSION (r104060): Web font is not loaded if specified by link element dyn...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 79312 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-21 19:34 PST by Yuzo Fujishima
Modified: 2012-02-23 13:12 PST (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Yuzo Fujishima 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>
Comment 1 Yuzo Fujishima 2012-02-21 19:35:36 PST
This can be related to http://wkb.ug/79021
Comment 2 Yuzo Fujishima 2012-02-21 19:38:51 PST
Created attachment 128115 [details]
Actual image
Comment 3 Yuzo Fujishima 2012-02-21 19:39:16 PST
Created attachment 128116 [details]
Expected image
Comment 4 Yuzo Fujishima 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.
Comment 5 Yuzo Fujishima 2012-02-21 21:35:41 PST
Sorry, Antti, not Annti.
Comment 6 Yuzo Fujishima 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)
Comment 7 Alexey Proskuryakov 2012-02-22 09:58:41 PST
<rdar://problem/10911246>
Comment 8 Antti Koivisto 2012-02-22 15:55:18 PST
Created attachment 128311 [details]
patch
Comment 9 Ryosuke Niwa 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.
Comment 10 Alexey Proskuryakov 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.
Comment 11 Antti Koivisto 2012-02-22 16:22:15 PST
Created attachment 128323 [details]
now with the test case included
Comment 12 Andreas Kling 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.
Comment 13 Antti Koivisto 2012-02-22 16:42:45 PST
http://trac.webkit.org/changeset/108574
Comment 14 Antti Koivisto 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.
Comment 15 Sean McBride 2012-02-23 13:12:57 PST
*** Bug 79312 has been marked as a duplicate of this bug. ***