<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[188634] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/188634">188634</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2015-08-18 23:23:41 -0700 (Tue, 18 Aug 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>[Cocoa] Punctuation near Hindi text is garbled when styled with the system font
https://bugs.webkit.org/show_bug.cgi?id=148164

Reviewed by Brian Burg.

Source/WebCore:

Fonts cache whether or not they are the system font. This caching took place at the end of Font::platformInit().
However, in the middle of Font::platformInit(), we look up a glyph, which calls GlyphPage::fill() which consults
with this cache. However, at this point, the cache has not been constructed yet. The solution is just to
construct the cache earlier (at the beginning of the function).

Consulting with the cache before it is populated causes it to erroneously say that no fonts are system fonts.
Then, we use Core Graphics to ask for glyphs instead of Core Text. Core Graphics, however, is incapable of
handling the system font, and returns us garbled results. In particular, when the system language is set to
Japanese, the system font does not support punctuation, and Core Text tells us so. However, Core Graphics
erroneously tells us that the system font does support punctuation.

Then, if text is near the punctuation which causes us to take the complex text codepath (such as Hindi text),
we tell Core Text to explicitly lay out the punctuation using the system font (which does not support
punctuation). Core Text then replies that the provided font doesn't support the punctuation, and that we should
use LastResort with some other glyphs instead. WebKit then disregards the font CoreText told us to use (because
we are oh-so-sure that the font in question supports punctuation) and uses the LastResort glyph IDs with our
font, which causes arbitrary glyphs to be shown.

Test: fast/text/hindi-system-font-punctuation.html

* platform/graphics/cocoa/FontCocoa.mm:
(WebCore::Font::platformInit):

LayoutTests:

This test is only relevant when the system font is set to Japanese or Simplified Chinese. In these
languages, the system font doesn't support punctuation, but CG will erroneously say that it does.

I intend to implement testing infrastructure which will allow us to mock the system language,
thereby allowing this test to be valid on all machines. The tracking bug for this effort is
https://bugs.webkit.org/show_bug.cgi?id=148168

* fast/text/hindi-system-font-punctuation-expected.html: Added.
* fast/text/hindi-system-font-punctuation.html: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaFontCocoamm">trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttexthindisystemfontpunctuationexpectedhtml">trunk/LayoutTests/fast/text/hindi-system-font-punctuation-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasttexthindisystemfontpunctuationhtml">trunk/LayoutTests/fast/text/hindi-system-font-punctuation.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (188633 => 188634)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2015-08-19 05:52:28 UTC (rev 188633)
+++ trunk/LayoutTests/ChangeLog        2015-08-19 06:23:41 UTC (rev 188634)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2015-08-18  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
+        [Cocoa] Punctuation near Hindi text is garbled when styled with the system font
+        https://bugs.webkit.org/show_bug.cgi?id=148164
+
+        Reviewed by Brian Burg.
+
+        This test is only relevant when the system font is set to Japanese or Simplified Chinese. In these
+        languages, the system font doesn't support punctuation, but CG will erroneously say that it does.
+
+        I intend to implement testing infrastructure which will allow us to mock the system language,
+        thereby allowing this test to be valid on all machines. The tracking bug for this effort is
+        https://bugs.webkit.org/show_bug.cgi?id=148168
+
+        * fast/text/hindi-system-font-punctuation-expected.html: Added.
+        * fast/text/hindi-system-font-punctuation.html: Added.
+
</ins><span class="cx"> 2015-08-18  Brian Burg  &lt;bburg@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [Win] Test Gardening after r188598
</span></span></pre></div>
<a id="trunkLayoutTestsfasttexthindisystemfontpunctuationexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/hindi-system-font-punctuation-expected.html (0 => 188634)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/hindi-system-font-punctuation-expected.html                                (rev 0)
+++ trunk/LayoutTests/fast/text/hindi-system-font-punctuation-expected.html        2015-08-19 06:23:41 UTC (rev 188634)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+&lt;html&gt;
+&lt;body style=&quot;font-family: -apple-system; text-rendering: optimizeSpeed;&quot;&gt;
+This test makes sure that punctuation next to Hindi characters are rendered as expected when the system language is set to Japanese.
+The test passes when the character below looks like a &quot;(&quot; character.
+&lt;div style=&quot;overflow: hidden; width: 40px; height: 400px;&quot;&gt;
+&lt;div style=&quot;width: 1000px; height: 1000px;&quot;&gt;
+&lt;div style=&quot;font-size: 200px;&quot;&gt;(&lt;span style=&quot;display: inline-block; height: 300px; width: 300px;&quot;&gt;&lt;/span&gt;&lt;/div&gt;
+&lt;/div&gt;
+&lt;/div&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttexthindisystemfontpunctuationhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/hindi-system-font-punctuation.html (0 => 188634)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/hindi-system-font-punctuation.html                                (rev 0)
+++ trunk/LayoutTests/fast/text/hindi-system-font-punctuation.html        2015-08-19 06:23:41 UTC (rev 188634)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+&lt;html&gt;
+&lt;body style=&quot;font-family: -apple-system; text-rendering: optimizeSpeed;&quot;&gt;
+This test makes sure that punctuation next to Hindi characters are rendered as expected when the system language is set to Japanese.
+The test passes when the character below looks like a &quot;(&quot; character.
+&lt;div style=&quot;overflow: hidden; width: 40px; height: 400px;&quot;&gt;&lt;!-- Covers up the &quot;&amp;#x928;)&quot; --&gt;
+&lt;div style=&quot;width: 1000px; height: 1000px;&quot;&gt;
+&lt;div style=&quot;font-size: 200px;&quot;&gt;(&amp;#x928;)&lt;span style=&quot;display: inline-block; height: 300px; width: 300px;&quot;&gt;&lt;/span&gt;&lt;/div&gt;
+&lt;/div&gt;
+&lt;/div&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (188633 => 188634)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-08-19 05:52:28 UTC (rev 188633)
+++ trunk/Source/WebCore/ChangeLog        2015-08-19 06:23:41 UTC (rev 188634)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2015-08-18  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
+        [Cocoa] Punctuation near Hindi text is garbled when styled with the system font
+        https://bugs.webkit.org/show_bug.cgi?id=148164
+
+        Reviewed by Brian Burg.
+
+        Fonts cache whether or not they are the system font. This caching took place at the end of Font::platformInit().
+        However, in the middle of Font::platformInit(), we look up a glyph, which calls GlyphPage::fill() which consults
+        with this cache. However, at this point, the cache has not been constructed yet. The solution is just to
+        construct the cache earlier (at the beginning of the function).
+
+        Consulting with the cache before it is populated causes it to erroneously say that no fonts are system fonts.
+        Then, we use Core Graphics to ask for glyphs instead of Core Text. Core Graphics, however, is incapable of
+        handling the system font, and returns us garbled results. In particular, when the system language is set to
+        Japanese, the system font does not support punctuation, and Core Text tells us so. However, Core Graphics
+        erroneously tells us that the system font does support punctuation.
+
+        Then, if text is near the punctuation which causes us to take the complex text codepath (such as Hindi text),
+        we tell Core Text to explicitly lay out the punctuation using the system font (which does not support
+        punctuation). Core Text then replies that the provided font doesn't support the punctuation, and that we should
+        use LastResort with some other glyphs instead. WebKit then disregards the font CoreText told us to use (because
+        we are oh-so-sure that the font in question supports punctuation) and uses the LastResort glyph IDs with our
+        font, which causes arbitrary glyphs to be shown.
+
+        Test: fast/text/hindi-system-font-punctuation.html
+
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::Font::platformInit):
+
</ins><span class="cx"> 2015-08-18  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Links for rules in &lt;style&gt; are incorrect, do not account for &lt;style&gt; offset in the document
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaFontCocoamm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm (188633 => 188634)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm        2015-08-19 05:52:28 UTC (rev 188633)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm        2015-08-19 06:23:41 UTC (rev 188634)
</span><span class="lines">@@ -93,6 +93,7 @@
</span><span class="cx"> 
</span><span class="cx"> void Font::platformInit()
</span><span class="cx"> {
</span><ins>+    // FIXME: Unify these two codepaths
</ins><span class="cx"> #if USE(APPKIT)
</span><span class="cx">     m_syntheticBoldOffset = m_platformData.m_syntheticBold ? 1.0f : 0.f;
</span><span class="cx"> 
</span><span class="lines">@@ -149,6 +150,8 @@
</span><span class="cx">         LOG_ERROR(&quot;failed to set up font, using system font %s&quot;, m_platformData.font());
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    m_isSystemFont = CTFontDescriptorIsSystemUIFont(adoptCF(CTFontCopyFontDescriptor(m_platformData.font())).get());
+
</ins><span class="cx">     // Work around &lt;rdar://problem/19433490&gt;
</span><span class="cx">     CGGlyph dummyGlyphs[] = {0, 0};
</span><span class="cx">     CGSize dummySize[] = { CGSizeMake(0, 0), CGSizeMake(0, 0) };
</span><span class="lines">@@ -215,7 +218,10 @@
</span><span class="cx">     m_fontMetrics.setCapHeight(capHeight);
</span><span class="cx">     m_fontMetrics.setLineGap(lineGap);
</span><span class="cx">     m_fontMetrics.setXHeight(xHeight);
</span><ins>+
</ins><span class="cx"> #else
</span><ins>+
+    m_isSystemFont = CTFontDescriptorIsSystemUIFont(adoptCF(CTFontCopyFontDescriptor(m_platformData.font())).get());
</ins><span class="cx">     m_syntheticBoldOffset = m_platformData.m_syntheticBold ? ceilf(m_platformData.size()  / 24.0f) : 0.f;
</span><span class="cx">     m_spaceGlyph = 0;
</span><span class="cx">     m_spaceWidth = 0;
</span><span class="lines">@@ -238,6 +244,7 @@
</span><span class="cx">         unitsPerEm = fontService.unitsPerEm();
</span><span class="cx">         familyName = adoptCF(CTFontCopyFamilyName(ctFont));
</span><span class="cx">     } else {
</span><ins>+        // FIXME: This else block is dead code. Remove it.
</ins><span class="cx">         CGFontRef cgFont = m_platformData.cgFont();
</span><span class="cx"> 
</span><span class="cx">         unitsPerEm = CGFontGetUnitsPerEm(cgFont);
</span><span class="lines">@@ -272,8 +279,6 @@
</span><span class="cx">         m_fontMetrics.setLineGap(thirdOfSize);
</span><span class="cx">     }
</span><span class="cx"> #endif
</span><del>-
-    m_isSystemFont = CTFontDescriptorIsSystemUIFont(adoptCF(CTFontCopyFontDescriptor(m_platformData.font())).get());
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void Font::platformCharWidthInit()
</span></span></pre>
</div>
</div>

</body>
</html>