Changeset 84264 in webkit


Ignore:
Timestamp:
Apr 19, 2011 10:40:45 AM (13 years ago)
Author:
jshin@chromium.org
Message:

Index: Source/JavaScriptCore/ChangeLog
===================================================================
--- Source/JavaScriptCore/ChangeLog (revision 84100)
+++ Source/JavaScriptCore/ChangeLog (working copy)
@@ -1,3 +1,15 @@
+2011-04-17 Jungshik Shin <jshin@chromium.org>
+
+ Reviewed by NOBODY (OOPS!).
+
+ Add U+FEFF (Zero width no-break space) to CharacterNames.h.
+ It's added to the list of characters to treat as zero-width
+ in WebCore.
+
+ https://bugs.webkit.org/show_bug.cgi?id=48860
+
+ * wtf/unicode/CharacterNames.h:
+

2011-04-16 Patrick Gansterer <Patrick Gansterer>


Reviewed by Eric Seidel.

Index: Source/JavaScriptCore/wtf/unicode/CharacterNames.h
===================================================================
--- Source/JavaScriptCore/wtf/unicode/CharacterNames.h (revision 84099)
+++ Source/JavaScriptCore/wtf/unicode/CharacterNames.h (working copy)
@@ -85,6 +85,7 @@ const UChar yenSign = 0x00A5;

const UChar zeroWidthJoiner = 0x200D;
const UChar zeroWidthNonJoiner = 0x200C;
const UChar zeroWidthSpace = 0x200B;

+const UChar zeroWidthNoBreakSpace = 0xFEFF;

} namespace Unicode
}
namespace WTF

@@ -138,5 +139,6 @@ using WTF::Unicode::yenSign;

using WTF::Unicode::zeroWidthJoiner;
using WTF::Unicode::zeroWidthNonJoiner;
using WTF::Unicode::zeroWidthSpace;

+using WTF::Unicode::zeroWidthNoBreakSpace;

#endif CharacterNames_h

Index: Source/WebCore/ChangeLog
===================================================================
--- Source/WebCore/ChangeLog (revision 84100)
+++ Source/WebCore/ChangeLog (working copy)
@@ -1,3 +1,37 @@
+2011-04-17 Jungshik Shin <jshin@chromium.org>
+
+ Reviewed by NOBODY (OOPS!).
+
+ Make U+FEFF be treated as a zero-width character in both
+ simple script and complex script code paths. In Chromium
+ Windows, UniscribeHelper needs a rather extensive changes
+ summarized below. Other ports need minor changes.
+
+ https://bugs.webkit.org/show_bug.cgi?id=48860
+
+ Test: fast/text/zero-width-characters-complex-script.html
+
+ * platform/graphics/Font.h:
+ (WebCore::Font::treatAsZeroWidthSpace): U+FEFF is added to the list
+ (WebCore::Font::treatAsZeroWidthSpaceInComplexScript): Added. Same as the above except that ZWNJ and ZWJ are excluded.
+ * platform/graphics/GlyphPageTreeNode.cpp:
+ (WebCore::GlyphPageTreeNode::initializePage): U+FEFF is made to have zero-width characters in simple script (fast) code path.
+ * platform/graphics/chromium/FontUtilsChromiumWin.cpp:
+ (WebCore::FontMap::getSpaceGlyph): Added to get the gid for space glyph to use in adjustSpaceAdvance when zero-width glyph character has a non-zero width and potentially 'visible' glyph.
+ (WebCore::FontMap::FontData::FontData): spaceGlyph member added.
+ (WebCore::getDerivedFontData): spaceGlyph is retrieved as well.
+ * platform/graphics/chromium/FontUtilsChromiumWin.h:
+ * platform/graphics/chromium/UniscribeHelper.cpp:
+ (WebCore::UniscribeHelper::UniscribeHelper): m_spaceGlyph added.
+ (WebCore::UniscribeHelper::shape): spaceGlyph is obtained stored for a font tried for each item.
+ (WebCore::UniscribeHelper::adjustSpaceAdvances): For zero-width complex script characters, set the advance width to zero and replace a non-zero-width/visible glyph with a space glyph.
+ (WebCore::UniscribeHelper::applySpacing):
+ (WebCore::UniscribeHelper::containsMissingGlyphs): turned to a member function because it cannot work on glyphs alone any more but need to take into account a character corresponding to a glyph
+ * platform/graphics/chromium/UniscribeHelper.h:
+ (WebCore::UniscribeHelper::Shaping::Shaping): m_spaceGlyph is added
+ * platform/graphics/chromium/UniscribeHelperTextRun.cpp:
+ (WebCore::UniscribeHelperTextRun::UniscribeHelperTextRun): When calling UniscriberHelper, add a new argument for spaceGlyph.
+

2011-04-16 Adam Barth <abarth@webkit.org>


Reviewed by Sam Weinig.

Index: Source/WebCore/platform/graphics/Font.h
===================================================================
--- Source/WebCore/platform/graphics/Font.h (revision 84099)
+++ Source/WebCore/platform/graphics/Font.h (working copy)
@@ -203,7 +203,8 @@ public:

FontSelector* fontSelector() const;

static bool treatAsSpace(UChar c) { return c == ' '
c == '\t' c == '\n' c == noBreakSpace; }
static bool treatAsZeroWidthSpace(UChar c) { return c < 0x20 + static bool treatAsZeroWidthSpace(UChar c) { return treatAsZeroWidthSpaceInComplexScript(c) + static bool treatAsZeroWidthSpaceInComplexScript(UChar c) { return c < 0x20
(c >= 0x7F && c < 0xA0) c == softHyphen (c >= 0x200c && c <= 0x200f) (c >= 0x202a && c <= 0x202e) c == objectReplacementCharacter; }
c == 0x200c c == 0x200d; }
(c >= 0x7F && c < 0xA0) c == softHyphen (c >= 0x200e && c <= 0x200f) (c >= 0x202a && c <= 0x202e) c == zeroWidthNoBreakSpace c == objectReplacementCharacter; }

static bool canReceiveTextEmphasis(UChar32 c);


static inline UChar normalizeSpaces(UChar character)

Index: Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp
===================================================================
--- Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (revision 84099)
+++ Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (working copy)
@@ -191,6 +191,9 @@ void GlyphPageTreeNode::initializePage(c

} else if (start == (objectReplacementCharacter & ~(GlyphPage::size - 1))) {

Object replacement character must not render at all.
buffer[objectReplacementCharacter - start] = zeroWidthSpace;

+ } else if (start == (zeroWidthNoBreakSpace & ~(GlyphPage::size - 1))) {
+ ZWNBS/BOM must not render at all.
+ buffer[zeroWidthNoBreakSpace - start] = zeroWidthSpace;

}

} else {

bufferLength = GlyphPage::size * 2;

Index: Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp
===================================================================
--- Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp (revision 84099)
+++ Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp (working copy)
@@ -250,17 +250,31 @@ int getAscent(HFONT hfont)

return gotMetrics ? tm.tmAscent : kUndefinedAscent;

}


+WORD getSpaceGlyph(HFONT hfont)
+{
+ HDC dc = GetDC(0);
+ HGDIOBJ oldFont = SelectObject(dc, hfont);
+ WCHAR space = L' ';
+ WORD spaceGlyph = 0;
+ GetGlyphIndices(dc, &space, 1, &spaceGlyph, 0);
+ SelectObject(dc, oldFont);
+ ReleaseDC(0, dc);
+ return spaceGlyph;
+}
+

struct FontData {

FontData()

: hfont(0)
, ascent(kUndefinedAscent)
, scriptCache(0)

+ , spaceGlyph(0)

{
}


HFONT hfont;
int ascent;
mutable SCRIPT_CACHE scriptCache;

+ WORD spaceGlyph;

};


Again, using hash_map does not earn us much here. page_cycler_test intl2

@@ -379,7 +393,8 @@ bool getDerivedFontData(const UChar* fam

LOGFONT* logfont,
int* ascent,
HFONT* hfont,

  • SCRIPT_CACHE scriptCache)

+ SCRIPT_CACHE scriptCache,
+ WORD* spaceGlyph)

{

ASSERT(logfont);
ASSERT(family);

@@ -408,6 +423,7 @@ bool getDerivedFontData(const UChar* fam

cache it so that we won't have to call CreateFontIndirect once
more for HFONT next time.
derived->ascent = getAscent(derived->hfont);

+ derived->spaceGlyph = getSpaceGlyph(derived->hfont);

} else {

derived = &iter->second;
Last time, GetAscent failed so that only HFONT was

@@ -419,6 +435,7 @@ bool getDerivedFontData(const UChar* fam

*hfont = derived->hfont;
*ascent = derived->ascent;
*scriptCache = &(derived->scriptCache);

+ *spaceGlyph = derived->spaceGlyph;

return *ascent != kUndefinedAscent;

}


Index: Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.h
===================================================================
--- Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.h (revision 84099)
+++ Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.h (working copy)
@@ -78,7 +78,7 @@ const UChar* getFallbackFamily(const UCh

intl2 page-cycler test is noticeably slower with one out param than
the current version although the subsequent 9 passes take about the
same time.

-bool getDerivedFontData(const UChar* family, int style, LOGFONT*, int* ascent, HFONT*, SCRIPT_CACHE);
+bool getDerivedFontData(const UChar* family, int style, LOGFONT*, int* ascent, HFONT*, SCRIPT_CACHE
, WORD* spaceGlyph);

enum {

FontStyleNormal = 0,

Index: Source/WebCore/platform/graphics/chromium/UniscribeHelper.cpp
===================================================================
--- Source/WebCore/platform/graphics/chromium/UniscribeHelper.cpp (revision 84099)
+++ Source/WebCore/platform/graphics/chromium/UniscribeHelper.cpp (working copy)
@@ -31,45 +31,16 @@

#include "config.h"
#include "UniscribeHelper.h"


-#include <windows.h>
-
+#include "Font.h"

#include "FontUtilsChromiumWin.h"
#include "PlatformContextSkia.h"
#include "SkiaFontWin.h"
#include "SkPoint.h"

+#include <windows.h>

#include <wtf/Assertions.h>


namespace WebCore {


- This function is used to see where word spacing should be applied inside
-
runs. Note that this must match Font::treatAsSpace so we all agree where
- and how much space this is, so we don't want to do more general Unicode
-
"is this a word break" thing.
-static bool treatAsSpace(UChar c)
-{

return c == ' '
c == '\t' c == '\n' c == 0x00A0;

-}
-
- SCRIPT_FONTPROPERTIES contains glyph indices for default, invalid
-
and blank glyphs. Just because ScriptShape succeeds does not mean
- that a text run is rendered correctly. Some characters may be rendered
-
with default/invalid/blank glyphs. Therefore, we need to check if the glyph
- array returned by ScriptShape contains any of those glyphs to make
-
sure that the text run is rendered successfully.
-static bool containsMissingGlyphs(WORD *glyphs,

  • int length,
  • SCRIPT_FONTPROPERTIES* properties)

-{

  • for (int i = 0; i < length; ++i) {
  • if (glyphs[i] == properties->wgDefault
(glyphs[i] == properties->wgInvalid
  • && glyphs[i] != properties->wgBlank))
  • return true;
  • }

-

  • return false;

-}
-

HFONT is the 'incarnation' of 'everything' about font, but it's an opaque
handle and we can't directly query it to make a new HFONT sharing
its characteristics (height, style, etc) except for family name.

@@ -102,13 +73,15 @@ UniscribeHelper::UniscribeHelper(const U

bool isRtl,
HFONT hfont,
SCRIPT_CACHE* scriptCache,

  • SCRIPT_FONTPROPERTIES* fontProperties)

+ SCRIPT_FONTPROPERTIES* fontProperties,
+ WORD spaceGlyph)

: m_input(input)
, m_inputLength(inputLength)
, m_isRtl(isRtl)
, m_hfont(hfont)
, m_scriptCache(scriptCache)
, m_fontProperties(fontProperties)

+ , m_spaceGlyph(spaceGlyph)

, m_directionalOverride(false)
, m_inhibitLigate(false)
, m_letterSpacing(0)

@@ -546,6 +519,7 @@ bool UniscribeHelper::shape(const UChar*

SCRIPT_CACHE* scriptCache = m_scriptCache;
SCRIPT_FONTPROPERTIES* fontProperties = m_fontProperties;
int ascent = m_ascent;

+ WORD spaceGlyph = m_spaceGlyph;

HDC tempDC = 0;
HGDIOBJ oldFont = 0;
HRESULT hr;

@@ -601,7 +575,7 @@ bool UniscribeHelper::shape(const UChar*

} else if (hr == E_OUTOFMEMORY) {

numGlyphs *= 2;
continue;

} else if (SUCCEEDED(hr) && (lastFallbackTried + } else if (SUCCEEDED(hr) && (lastFallbackTried
!containsMissingGlyphs(&shaping.m_glyphs[0], generatedGlyphs, fontProperties)))
!containsMissingGlyphs(shaping, run, fontProperties)))

break;


The current font can't render this run. clear DC and try

@@ -632,7 +606,9 @@ bool UniscribeHelper::shape(const UChar*

const UChar *family = getFallbackFamily(input, itemLength,

FontDescription::StandardFamily, 0, 0);

bool fontOk = getDerivedFontData(family, m_style, &m_logfont,

  • &ascent, &hfont, &scriptCache);

+ &ascent, &hfont, &scriptCache,
+ &spaceGlyph);
+

if (!fontOk) {

If this GetDerivedFontData is called from the renderer it

@@ -644,7 +620,8 @@ bool UniscribeHelper::shape(const UChar*

Try again.
fontOk = getDerivedFontData(family, m_style, &m_logfont,

  • &ascent, &hfont, &scriptCache);

+ &ascent, &hfont, &scriptCache,
+ &spaceGlyph);

ASSERT(fontOk);

}


@@ -673,6 +650,7 @@ bool UniscribeHelper::shape(const UChar*

because it's not used elsewhere.
shaping.m_hfont = hfont;
shaping.m_scriptCache = scriptCache;

+ shaping.m_spaceGlyph = spaceGlyph;

The ascent of a font for this run can be different from
that of the primary font so that we need to keep track of

@@ -806,22 +784,39 @@ void UniscribeHelper::adjustSpaceAdvance

for (size_t run = 0; run < m_runs.size(); run++) {

Shaping& shaping = m_shapes[run];


+ FIXME: This loop is not UTF-16-safe. Unicode 6.0 has a couple
+
of complex script blocks in Plane 1.

for (int i = 0; i < shaping.charLength(); i++) {

  • if (!treatAsSpace(m_input[m_runs[run].iCharPos + i]))

+ UChar c = m_input[m_runs[run].iCharPos + i];
+ bool treatAsSpace = Font::treatAsSpace(c);
+ if (!treatAsSpace && !Font::treatAsZeroWidthSpaceInComplexScript(c))

continue;


int glyphIndex = shaping.m_logs[i];
int currentAdvance = shaping.m_advance[glyphIndex];


  • currentAdvance does not include additional letter-spacing, but
  • space_width does. Here we find out how off we are from the
  • correct width for the space not including letter-spacing, then
  • just subtract that diff.
  • int diff = currentAdvance - spaceWidthWithoutLetterSpacing;
  • The shaping can consist of a run of text, so only subtract the
  • difference in the width of the glyph.
  • shaping.m_advance[glyphIndex] -= diff;
  • shaping.m_abc.abcB -= diff;

+ if (treatAsSpace) {
+ currentAdvance does not include additional letter-spacing,
+
but m_spaceWidth does. Here we find out how off we are from
+ the correct width (spaceWidthWithoutLetterSpacing) and
+
just subtract that diff.
+ int diff = currentAdvance - spaceWidthWithoutLetterSpacing;
+ The shaping can consist of a run of text, so only subtract
+
the difference in the width of the glyph.
+ shaping.m_advance[glyphIndex] -= diff;
+ shaping.m_abc.abcB -= diff;
+ continue;
+ }
+
+ For characters treated as zero-width space in complex
+
scripts, set the advance width to zero, adjust
+ |abcB| of the current run accordingly and set
+
the glyph to m_spaceGlyph (invisible).
+ shaping.m_advance[glyphIndex] = 0;
+ shaping.m_abc.abcB -= currentAdvance;
+ shaping.m_offsets[glyphIndex].du = 0;
+ shaping.m_offsets[glyphIndex].dv = 0;
+ shaping.m_glyphs[glyphIndex] = shaping.m_spaceGlyph;

}

}

}

@@ -872,7 +867,7 @@ void UniscribeHelper::applySpacing()

extra wordspacing amount for the glyphs they correspond to.
if (m_wordSpacing != 0) {

for (int i = 0; i < shaping.charLength(); i++) {

  • if (!treatAsSpace(m_input[m_runs[run].iCharPos + i]))

+ if (!Font::treatAsSpace(m_input[m_runs[run].iCharPos + i]))

continue;


The char in question is a word separator...

@@ -929,4 +924,31 @@ int UniscribeHelper::advanceForItem(int

return shaping.m_prePadding + justification;

}


+ SCRIPT_FONTPROPERTIES contains glyph indices for default, invalid
+
and blank glyphs. Just because ScriptShape succeeds does not mean
+ that a text run is rendered correctly. Some characters may be rendered
+
with default/invalid/blank glyphs. Therefore, we need to check if the glyph
+ array returned by ScriptShape contains any of those glyphs to make
+
sure that the text run is rendered successfully.
+ However, we should not subject zero-width characters to this test.
+
+bool UniscribeHelper::containsMissingGlyphs(const Shaping& shaping,
+ const SCRIPT_ITEM& run,
+ const SCRIPT_FONTPROPERTIES* properties) const
+{
+ for (int i = 0; i < shaping.charLength(); i++) {
+ UChar c = m_input[run.iCharPos + i];
+
Skip zero-width space characters because they're not considered to be missing in a font.
+ if (Font::treatAsZeroWidthSpaceInComplexScript(c))
+ continue;
+ int glyphIndex = shaping.m_logs[i];
+ WORD glyph = shaping.m_glyphs[glyphIndex];
+ if (glyph == properties->wgDefault

+
(glyph == properties->wgInvalid && glyph != properties->wgBlank))

+ return true;
+ }
+ return false;
+}
+
+

} namespace WebCore

Index: Source/WebCore/platform/graphics/chromium/UniscribeHelper.h
===================================================================
--- Source/WebCore/platform/graphics/chromium/UniscribeHelper.h (revision 84099)
+++ Source/WebCore/platform/graphics/chromium/UniscribeHelper.h (working copy)
@@ -76,7 +76,8 @@ public:

bool isRtl,
HFONT,
SCRIPT_CACHE*,

  • SCRIPT_FONTPROPERTIES*);

+ SCRIPT_FONTPROPERTIES*,
+ WORD);

virtual ~UniscribeHelper();


@@ -225,7 +226,9 @@ private:

: m_prePadding(0)
, m_hfont(NULL)
, m_scriptCache(NULL)

  • , m_ascentOffset(0) {

+ , m_ascentOffset(0)
+ , m_spaceGlyph(0)
+ {

m_abc.abcA = 0;
m_abc.abcB = 0;
m_abc.abcC = 0;

@@ -319,6 +322,8 @@ private:

when drawing a string, to align multiple runs rendered with
different fonts.
int m_ascentOffset;

+
+ WORD m_spaceGlyph;

};


Computes the runs_ array from the text run.

@@ -343,6 +348,10 @@ private:

Returns the total width of a single item.
int advanceForItem(int) const;


+ bool containsMissingGlyphs(const Shaping&,
+ const SCRIPT_ITEM&,
+ const SCRIPT_FONTPROPERTIES*) const;
+

Shapes a run (pointed to by |input|) using |hfont| first.
Tries a series of fonts specified retrieved with NextWinFontData
and finally a font covering characters in |*input|. A string pointed

@@ -384,6 +393,7 @@ private:

int m_ascent;
LOGFONT m_logfont;
int m_style;

+ WORD m_spaceGlyph;

Options, see the getters/setters above.
bool m_directionalOverride;

Index: Source/WebCore/platform/graphics/chromium/UniscribeHelperTextRun.cpp
===================================================================
--- Source/WebCore/platform/graphics/chromium/UniscribeHelperTextRun.cpp (revision 84099)
+++ Source/WebCore/platform/graphics/chromium/UniscribeHelperTextRun.cpp (working copy)
@@ -43,7 +43,8 @@ UniscribeHelperTextRun::UniscribeHelperT

: UniscribeHelper(run.characters(), run.length(), run.rtl(),

font.primaryFont()->platformData().hfont(),
font.primaryFont()->platformData().scriptCache(),

  • font.primaryFont()->platformData().scriptFontProperties())

+ font.primaryFont()->platformData().scriptFontProperties(),
+ font.primaryFont()->spaceGlyph())

, m_font(&font)
, m_fontIndex(0)

{

@@ -69,7 +70,7 @@ UniscribeHelperTextRun::UniscribeHelperT

SCRIPT_CACHE* scriptCache,
SCRIPT_FONTPROPERTIES* fontProperties)
: UniscribeHelper(input, inputLength, isRtl, hfont,

  • scriptCache, fontProperties)

+ scriptCache, fontProperties, 0)

, m_font(0)
, m_fontIndex(-1)

{

Index: LayoutTests/ChangeLog
===================================================================
--- LayoutTests/ChangeLog (revision 84100)
+++ LayoutTests/ChangeLog (working copy)
@@ -1,3 +1,21 @@
+2011-04-17 Jungshik Shin <jshin@chromium.org>
+
+ Reviewed by NOBODY (OOPS!).
+
+ Add a complex-script version of zero-width-characters.html.
+ and add U+FEFF to zero-width-characters.html
+ Chromium Linux fails the test because U+FEFF is rendered
+ with a non-zero width glyph.
+ Filed http://bugs.webkit.org/show_bug.cgi?id=58741 and noted
+ as such in test_expectation.txt.
+
+ https://bugs.webkit.org/show_bug.cgi?id=48860
+
+ * fast/text/zero-width-characters-complex-script-expected.txt: Added.
+ * fast/text/zero-width-characters-complex-script.html: Added.
+ * fast/text/zero-width-characters.html:
+ * platform/chromium/test_expectations.txt:
+

2011-04-16 Dan Bernstein <mitz@apple.com>


Updated results for fast/block/float/032.html after r84096. Filed http://webkit.org/b/58736 to

Index: LayoutTests/fast/text/zero-width-characters-complex-script-expected.txt
===================================================================
--- LayoutTests/fast/text/zero-width-characters-complex-script-expected.txt (revision 0)
+++ LayoutTests/fast/text/zero-width-characters-complex-script-expected.txt (revision 0)
@@ -0,0 +1,3 @@
+This test checks various characters that should always be zero width to ensure that they are when enclosed by complex script characters. The WebKit text system ensures this in a way that's independent of the fonts installed on the system.
+
+PASS: All characters had zero-width.
Index: LayoutTests/fast/text/zero-width-characters-complex-script.html
===================================================================
--- LayoutTests/fast/text/zero-width-characters-complex-script.html (revision 0)
+++ LayoutTests/fast/text/zero-width-characters-complex-script.html (revision 0)
@@ -0,0 +1,66 @@
+<head>
+<script>
+
+function testChar(ch)
+{
+ Strings a and b selected here do not have any 'interaction' between them.
+ var a = "\u0915\u093E"
+ var b = "\u0916";
+ var span = document.getElementById("characters");
+ span.firstChild.data = a + b;
+ var abWidth = span.offsetWidth;
+ span.firstChild.data = a;
+ var aWidth = span.offsetWidth;
+ span.firstChild.data = a + String.fromCharCode(ch) + b;
+ var abWithChWidth = span.offsetWidth;
+
+ if (abWithChWidth > abWidth)
+ return 1;
+ if (abWidth > aWidth)
+ return 0;
+ return 1;
+}
+
+function test()
+{
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ var failedCount = 0;
+ for (var i = 1; i < 32; ++i)
+ if (i != 9 && i != 10 && i != 13)
+ failedCount += testChar(i);
+
+ for (var i = 0x7F; i < 0xA0; ++i)
+ failedCount += testChar(i);
+ failedCount += testChar(0xAD);
+
ZWJ (U+200C) and ZWNJ (U+200D) are excluded because they
+ can affect the rendering in complex script text.
+ failedCount += testChar(0x200B);
+ failedCount += testChar(0x200E);
+ failedCount += testChar(0x200F);
+ failedCount += testChar(0x202A);
+ failedCount += testChar(0x202B);
+ failedCount += testChar(0x202C);
+ failedCount += testChar(0x202D);
+ failedCount += testChar(0x202E);
+ failedCount += testChar(0xFEFF);
+ failedCount += testChar(0xFFFC);
+
+ var testArea = document.getElementById("testArea");
+ testArea.parentNode.removeChild(testArea);
+
+ if (failedCount > 0)
+ result = "FAIL: " + failedCount + " characters had non-zero width" +
+ " or failed to get measured.";
+ else
+ result = "PASS: All characters had zero-width.";
+ document.getElementById("result").firstChild.data = result;
+}
+</script>
+</head>
+<body onload="test()">
+<p>This test checks various characters that should always be zero width to ensure that they are when enclosed by complex script characters.
+The WebKit text system ensures this in a way that's independent of the fonts installed on the system.</p>
+<p id="result">FAIL: Script did not run to completion.</p>
+<p id="testArea"><span id="characters">&#x0915;&#x093E;&#x0916;</span></p>
+</body>
Index: LayoutTests/fast/text/zero-width-characters.html
===================================================================
--- LayoutTests/fast/text/zero-width-characters.html (revision 84099)
+++ LayoutTests/fast/text/zero-width-characters.html (working copy)
@@ -13,6 +13,7 @@ function test()

testString += String.fromCharCode(0x200D);
testString += String.fromCharCode(0x200E);
testString += String.fromCharCode(0x200F);

+ testString += String.fromCharCode(0xFEFF);

testString += String.fromCharCode(0xFFFC);
var span = document.getElementById("characters");
var abWidth = span.offsetWidth;

Index: LayoutTests/platform/chromium/test_expectations.txt
===================================================================
--- LayoutTests/platform/chromium/test_expectations.txt (revision 84099)
+++ LayoutTests/platform/chromium/test_expectations.txt (working copy)
@@ -3534,4 +3534,5 @@ BUGMORRITA : fast/html/details-remove-su

BUGMORRITA : fast/html/details-writing-mode.html = FAIL


BUGWK58619 GPU LINUX WIN : media/video-volume-slider.html = IMAGE

+BUGWK58741 LINUX : fast/text/zero-width-characters-complex-script.html = FAIL

Location:
trunk
Files:
2 added
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r84256 r84264  
     12011-04-19  Jungshik Shin  <jshin@chromium.org>
     2
     3        Reviewed by David Levin
     4
     5        Add a complex-script version of zero-width-characters.html.
     6        and add U+FEFF to zero-width-characters.html
     7        Chromium Linux fails the test because U+FEFF is rendered
     8        with a non-zero width glyph.
     9        Filed http://bugs.webkit.org/show_bug.cgi?id=58741 and noted
     10        as such in test_expectation.txt.
     11       
     12        https://bugs.webkit.org/show_bug.cgi?id=48860
     13
     14        * fast/text/zero-width-characters-complex-script-expected.txt: Added.
     15        * fast/text/zero-width-characters-complex-script.html: Added.
     16        * fast/text/zero-width-characters.html:
     17        * platform/chromium/test_expectations.txt:
     18
    1192011-04-19  Kristóf Kosztyó  <Kosztyo.Kristof@stud.u-szeged.hu>
    220
  • trunk/LayoutTests/fast/text/zero-width-characters.html

    r58042 r84264  
    1414    testString += String.fromCharCode(0x200E);
    1515    testString += String.fromCharCode(0x200F);
     16    testString += String.fromCharCode(0xFEFF);
    1617    testString += String.fromCharCode(0xFFFC);
    1718    var span = document.getElementById("characters");
  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r84255 r84264  
    34873487
    34883488BUGWK58619 GPU LINUX WIN : media/video-volume-slider.html = IMAGE
     3489BUGWK58741 LINUX : fast/text/zero-width-characters-complex-script.html = FAIL
    34893490
    34903491// Broken by skia shadow code change in r83936
  • trunk/Source/JavaScriptCore/ChangeLog

    r84252 r84264  
     12011-04-19  Jungshik Shin  <jshin@chromium.org>
     2
     3        Reviewed by David Levin
     4
     5        Add U+FEFF (Zero width no-break space) to CharacterNames.h.
     6        It's added to the list of characters to treat as zero-width
     7        in WebCore.
     8
     9        https://bugs.webkit.org/show_bug.cgi?id=48860
     10
     11        * wtf/unicode/CharacterNames.h:
     12
    1132011-04-19  Csaba Osztrogonác  <ossy@webkit.org>
    214
  • trunk/Source/JavaScriptCore/wtf/unicode/CharacterNames.h

    r77062 r84264  
    8686const UChar zeroWidthNonJoiner = 0x200C;
    8787const UChar zeroWidthSpace = 0x200B;
     88const UChar zeroWidthNoBreakSpace = 0xFEFF;
    8889
    8990} // namespace Unicode
     
    139140using WTF::Unicode::zeroWidthNonJoiner;
    140141using WTF::Unicode::zeroWidthSpace;
     142using WTF::Unicode::zeroWidthNoBreakSpace;
    141143
    142144#endif // CharacterNames_h
  • trunk/Source/WebCore/ChangeLog

    r84261 r84264  
     12011-04-19  Jungshik Shin  <jshin@chromium.org>
     2
     3        Reviewed by David Levin
     4
     5        Make U+FEFF be treated as a zero-width character in both
     6        simple script and complex script code paths. In Chromium
     7        Windows, UniscribeHelper needs a rather extensive changes
     8        summarized below.  Other ports need minor changes.
     9
     10        https://bugs.webkit.org/show_bug.cgi?id=48860
     11
     12        Test: fast/text/zero-width-characters-complex-script.html
     13
     14        * platform/graphics/Font.h:
     15        (WebCore::Font::treatAsZeroWidthSpace): U+FEFF is added to the list
     16        (WebCore::Font::treatAsZeroWidthSpaceInComplexScript): Added. Same as the above except that ZWNJ and ZWJ are excluded.
     17        * platform/graphics/GlyphPageTreeNode.cpp:
     18        (WebCore::GlyphPageTreeNode::initializePage): U+FEFF is made to have zero-width characters in simple script (fast) code path.
     19        * platform/graphics/chromium/FontUtilsChromiumWin.cpp:
     20        (WebCore::FontMap::getSpaceGlyph): Added to get the gid for space glyph to use in adjustSpaceAdvance when zero-width glyph character has a non-zero width and potentially 'visible' glyph.
     21        (WebCore::FontMap::FontData::FontData): spaceGlyph member added.
     22        (WebCore::getDerivedFontData): spaceGlyph is retrieved as well.
     23        * platform/graphics/chromium/FontUtilsChromiumWin.h:
     24        * platform/graphics/chromium/UniscribeHelper.cpp:
     25        (WebCore::UniscribeHelper::UniscribeHelper): m_spaceGlyph added.
     26        (WebCore::UniscribeHelper::shape): spaceGlyph is obtained stored for a font tried for each item.
     27        (WebCore::UniscribeHelper::adjustSpaceAdvances): For zero-width complex script characters, set the advance width to zero and replace a non-zero-width/visible glyph with a space glyph.
     28        (WebCore::UniscribeHelper::applySpacing):
     29        (WebCore::UniscribeHelper::containsMissingGlyphs): turned to a member function because it cannot work on glyphs alone any more but need to take into account a character corresponding to a glyph
     30        * platform/graphics/chromium/UniscribeHelper.h:
     31        (WebCore::UniscribeHelper::Shaping::Shaping): m_spaceGlyph is added
     32        * platform/graphics/chromium/UniscribeHelperTextRun.cpp:
     33        (WebCore::UniscribeHelperTextRun::UniscribeHelperTextRun): When calling UniscriberHelper, add a new argument for spaceGlyph.
     34
    1352011-04-19  Brent Fulgham  <bfulgham@webkit.org>
    236
  • trunk/Source/WebCore/platform/graphics/Font.h

    r81684 r84264  
    204204    FontSelector* fontSelector() const;
    205205    static bool treatAsSpace(UChar c) { return c == ' ' || c == '\t' || c == '\n' || c == noBreakSpace; }
    206     static bool treatAsZeroWidthSpace(UChar c) { return c < 0x20 || (c >= 0x7F && c < 0xA0) || c == softHyphen || (c >= 0x200c && c <= 0x200f) || (c >= 0x202a && c <= 0x202e) || c == objectReplacementCharacter; }
     206    static bool treatAsZeroWidthSpace(UChar c) { return treatAsZeroWidthSpaceInComplexScript(c) || c == 0x200c || c == 0x200d; }
     207    static bool treatAsZeroWidthSpaceInComplexScript(UChar c) { return c < 0x20 || (c >= 0x7F && c < 0xA0) || c == softHyphen || (c >= 0x200e && c <= 0x200f) || (c >= 0x202a && c <= 0x202e) || c == zeroWidthNoBreakSpace || c == objectReplacementCharacter; }
    207208    static bool canReceiveTextEmphasis(UChar32 c);
    208209
  • trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp

    r77062 r84264  
    192192                    // Object replacement character must not render at all.
    193193                    buffer[objectReplacementCharacter - start] = zeroWidthSpace;
     194                } else if (start == (zeroWidthNoBreakSpace & ~(GlyphPage::size - 1))) {
     195                    // ZWNBS/BOM must not render at all.
     196                    buffer[zeroWidthNoBreakSpace - start] = zeroWidthSpace;
    194197                }
    195198            } else {
  • trunk/Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp

    r72498 r84264  
    251251}
    252252
     253WORD getSpaceGlyph(HFONT hfont)
     254{
     255    HDC dc = GetDC(0);
     256    HGDIOBJ oldFont = SelectObject(dc, hfont);
     257    WCHAR space = L' ';
     258    WORD spaceGlyph = 0;
     259    GetGlyphIndices(dc, &space, 1, &spaceGlyph, 0);
     260    SelectObject(dc, oldFont);
     261    ReleaseDC(0, dc);
     262    return spaceGlyph;
     263}
     264
    253265struct FontData {
    254266    FontData()
     
    256268        , ascent(kUndefinedAscent)
    257269        , scriptCache(0)
     270        , spaceGlyph(0)
    258271    {
    259272    }
     
    262275    int ascent;
    263276    mutable SCRIPT_CACHE scriptCache;
     277    WORD spaceGlyph;
    264278};
    265279
     
    380394                        int* ascent,
    381395                        HFONT* hfont,
    382                         SCRIPT_CACHE** scriptCache)
     396                        SCRIPT_CACHE** scriptCache,
     397                        WORD* spaceGlyph)
    383398{
    384399    ASSERT(logfont);
     
    409424        // more for HFONT next time.
    410425        derived->ascent = getAscent(derived->hfont);
     426        derived->spaceGlyph = getSpaceGlyph(derived->hfont);
    411427    } else {
    412428        derived = &iter->second;
     
    420436    *ascent = derived->ascent;
    421437    *scriptCache = &(derived->scriptCache);
     438    *spaceGlyph = derived->spaceGlyph;
    422439    return *ascent != kUndefinedAscent;
    423440}
  • trunk/Source/WebCore/platform/graphics/chromium/FontUtilsChromiumWin.h

    r61663 r84264  
    7979// the current version although the subsequent 9 passes take about the
    8080// same time.
    81 bool getDerivedFontData(const UChar* family, int style, LOGFONT*, int* ascent, HFONT*, SCRIPT_CACHE**);
     81bool getDerivedFontData(const UChar* family, int style, LOGFONT*, int* ascent, HFONT*, SCRIPT_CACHE**, WORD* spaceGlyph);
    8282
    8383enum {
  • trunk/Source/WebCore/platform/graphics/chromium/UniscribeHelper.cpp

    r57595 r84264  
    3232#include "UniscribeHelper.h"
    3333
    34 #include <windows.h>
    35 
     34#include "Font.h"
    3635#include "FontUtilsChromiumWin.h"
    3736#include "PlatformContextSkia.h"
    3837#include "SkiaFontWin.h"
    3938#include "SkPoint.h"
     39#include <windows.h>
    4040#include <wtf/Assertions.h>
    4141
    4242namespace WebCore {
    43 
    44 // This function is used to see where word spacing should be applied inside
    45 // runs. Note that this must match Font::treatAsSpace so we all agree where
    46 // and how much space this is, so we don't want to do more general Unicode
    47 // "is this a word break" thing.
    48 static bool treatAsSpace(UChar c)
    49 {
    50     return c == ' ' || c == '\t' || c == '\n' || c == 0x00A0;
    51 }
    52 
    53 // SCRIPT_FONTPROPERTIES contains glyph indices for default, invalid
    54 // and blank glyphs. Just because ScriptShape succeeds does not mean
    55 // that a text run is rendered correctly. Some characters may be rendered
    56 // with default/invalid/blank glyphs. Therefore, we need to check if the glyph
    57 // array returned by ScriptShape contains any of those glyphs to make
    58 // sure that the text run is rendered successfully.
    59 static bool containsMissingGlyphs(WORD *glyphs,
    60                                   int length,
    61                                   SCRIPT_FONTPROPERTIES* properties)
    62 {
    63     for (int i = 0; i < length; ++i) {
    64         if (glyphs[i] == properties->wgDefault
    65             || (glyphs[i] == properties->wgInvalid
    66             && glyphs[i] != properties->wgBlank))
    67             return true;
    68     }
    69 
    70     return false;
    71 }
    7243
    7344// HFONT is the 'incarnation' of 'everything' about font, but it's an opaque
     
    10374                                HFONT hfont,
    10475                                SCRIPT_CACHE* scriptCache,
    105                                 SCRIPT_FONTPROPERTIES* fontProperties)
     76                                SCRIPT_FONTPROPERTIES* fontProperties,
     77                                WORD spaceGlyph)
    10678    : m_input(input)
    10779    , m_inputLength(inputLength)
     
    11082    , m_scriptCache(scriptCache)
    11183    , m_fontProperties(fontProperties)
     84    , m_spaceGlyph(spaceGlyph)
    11285    , m_directionalOverride(false)
    11386    , m_inhibitLigate(false)
     
    547520    SCRIPT_FONTPROPERTIES* fontProperties = m_fontProperties;
    548521    int ascent = m_ascent;
     522    WORD spaceGlyph = m_spaceGlyph;
    549523    HDC tempDC = 0;
    550524    HGDIOBJ oldFont = 0;
     
    602576            numGlyphs *= 2;
    603577            continue;
    604         } else if (SUCCEEDED(hr) && (lastFallbackTried || !containsMissingGlyphs(&shaping.m_glyphs[0], generatedGlyphs, fontProperties)))
     578        } else if (SUCCEEDED(hr) && (lastFallbackTried || !containsMissingGlyphs(shaping, run, fontProperties)))
    605579            break;
    606580
     
    633607                FontDescription::StandardFamily, 0, 0);
    634608            bool fontOk = getDerivedFontData(family, m_style, &m_logfont,
    635                                               &ascent, &hfont, &scriptCache);
     609                                             &ascent, &hfont, &scriptCache,
     610                                             &spaceGlyph);
     611                                             
    636612
    637613            if (!fontOk) {
     
    645621                // Try again.
    646622                fontOk = getDerivedFontData(family, m_style, &m_logfont,
    647                                              &ascent, &hfont, &scriptCache);
     623                                            &ascent, &hfont, &scriptCache,
     624                                            &spaceGlyph);
    648625                ASSERT(fontOk);
    649626            }
     
    674651    shaping.m_hfont = hfont;
    675652    shaping.m_scriptCache = scriptCache;
     653    shaping.m_spaceGlyph = spaceGlyph;
    676654
    677655    // The ascent of a font for this run can be different from
     
    807785        Shaping& shaping = m_shapes[run];
    808786
     787        // FIXME: This loop is not UTF-16-safe. Unicode 6.0 has a couple
     788        // of complex script blocks in Plane 1.
    809789        for (int i = 0; i < shaping.charLength(); i++) {
    810             if (!treatAsSpace(m_input[m_runs[run].iCharPos + i]))
     790            UChar c = m_input[m_runs[run].iCharPos + i];
     791            bool treatAsSpace = Font::treatAsSpace(c);
     792            if (!treatAsSpace && !Font::treatAsZeroWidthSpaceInComplexScript(c))
    811793                continue;
    812794
     
    814796            int currentAdvance = shaping.m_advance[glyphIndex];
    815797
    816             // currentAdvance does not include additional letter-spacing, but
    817             // space_width does. Here we find out how off we are from the
    818             // correct width for the space not including letter-spacing, then
    819             // just subtract that diff.
    820             int diff = currentAdvance - spaceWidthWithoutLetterSpacing;
    821             // The shaping can consist of a run of text, so only subtract the
    822             // difference in the width of the glyph.
    823             shaping.m_advance[glyphIndex] -= diff;
    824             shaping.m_abc.abcB -= diff;
     798            if (treatAsSpace) {
     799                // currentAdvance does not include additional letter-spacing,
     800                // but m_spaceWidth does. Here we find out how off we are from
     801                // the correct width (spaceWidthWithoutLetterSpacing) and
     802                // just subtract that diff.
     803                int diff = currentAdvance - spaceWidthWithoutLetterSpacing;
     804                // The shaping can consist of a run of text, so only subtract
     805                // the difference in the width of the glyph.
     806                shaping.m_advance[glyphIndex] -= diff;
     807                shaping.m_abc.abcB -= diff;
     808                continue;
     809            }
     810
     811            // For characters treated as zero-width space in complex
     812            // scripts, set the advance width to zero, adjust
     813            // |abcB| of the current run accordingly and set
     814            // the glyph to m_spaceGlyph (invisible).
     815            shaping.m_advance[glyphIndex] = 0;
     816            shaping.m_abc.abcB -= currentAdvance;
     817            shaping.m_offsets[glyphIndex].du = 0;
     818            shaping.m_offsets[glyphIndex].dv = 0;
     819            shaping.m_glyphs[glyphIndex] = shaping.m_spaceGlyph;
    825820        }
    826821    }
     
    873868        if (m_wordSpacing != 0) {
    874869            for (int i = 0; i < shaping.charLength(); i++) {
    875                 if (!treatAsSpace(m_input[m_runs[run].iCharPos + i]))
     870                if (!Font::treatAsSpace(m_input[m_runs[run].iCharPos + i]))
    876871                    continue;
    877872
     
    930925}
    931926
     927// SCRIPT_FONTPROPERTIES contains glyph indices for default, invalid
     928// and blank glyphs. Just because ScriptShape succeeds does not mean
     929// that a text run is rendered correctly. Some characters may be rendered
     930// with default/invalid/blank glyphs. Therefore, we need to check if the glyph
     931// array returned by ScriptShape contains any of those glyphs to make
     932// sure that the text run is rendered successfully.
     933// However, we should not subject zero-width characters to this test.
     934
     935bool UniscribeHelper::containsMissingGlyphs(const Shaping& shaping,
     936                                            const SCRIPT_ITEM& run,
     937                                            const SCRIPT_FONTPROPERTIES* properties) const
     938{
     939    for (int i = 0; i < shaping.charLength(); i++) {
     940        UChar c = m_input[run.iCharPos + i];
     941        // Skip zero-width space characters because they're not considered to be missing in a font.
     942        if (Font::treatAsZeroWidthSpaceInComplexScript(c))
     943            continue;
     944        int glyphIndex = shaping.m_logs[i];
     945        WORD glyph = shaping.m_glyphs[glyphIndex];
     946        if (glyph == properties->wgDefault
     947            || (glyph == properties->wgInvalid && glyph != properties->wgBlank))
     948            return true;
     949    }
     950    return false;
     951}
     952
     953
    932954}  // namespace WebCore
  • trunk/Source/WebCore/platform/graphics/chromium/UniscribeHelper.h

    r40812 r84264  
    7777                    HFONT,
    7878                    SCRIPT_CACHE*,
    79                     SCRIPT_FONTPROPERTIES*);
     79                    SCRIPT_FONTPROPERTIES*,
     80                    WORD);
    8081
    8182    virtual ~UniscribeHelper();
     
    226227            , m_hfont(NULL)
    227228            , m_scriptCache(NULL)
    228             , m_ascentOffset(0) {
     229            , m_ascentOffset(0)
     230            , m_spaceGlyph(0)
     231        {
    229232            m_abc.abcA = 0;
    230233            m_abc.abcB = 0;
     
    320323        // different fonts.
    321324        int m_ascentOffset;
     325
     326        WORD m_spaceGlyph;
    322327    };
    323328
     
    343348    // Returns the total width of a single item.
    344349    int advanceForItem(int) const;
     350
     351    bool containsMissingGlyphs(const Shaping&,
     352                               const SCRIPT_ITEM&,
     353                               const SCRIPT_FONTPROPERTIES*) const;
    345354
    346355    // Shapes a run (pointed to by |input|) using |hfont| first.
     
    385394    LOGFONT m_logfont;
    386395    int m_style;
     396    WORD m_spaceGlyph;
    387397
    388398    // Options, see the getters/setters above.
  • trunk/Source/WebCore/platform/graphics/chromium/UniscribeHelperTextRun.cpp

    r76743 r84264  
    4444                      font.primaryFont()->platformData().hfont(),
    4545                      font.primaryFont()->platformData().scriptCache(),
    46                       font.primaryFont()->platformData().scriptFontProperties())
     46                      font.primaryFont()->platformData().scriptFontProperties(),
     47                      font.primaryFont()->spaceGlyph())
    4748    , m_font(&font)
    4849    , m_fontIndex(0)
     
    7071    SCRIPT_FONTPROPERTIES* fontProperties)
    7172    : UniscribeHelper(input, inputLength, isRtl, hfont,
    72                       scriptCache, fontProperties)
     73                      scriptCache, fontProperties, 0)
    7374    , m_font(0)
    7475    , m_fontIndex(-1)
Note: See TracChangeset for help on using the changeset viewer.