diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 35f4168e1ef9..09b8e61e7712 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,21 @@ +2017-10-26 Carlos Garcia Campos + + REGRESSION(r222090): [HarfBuzz] Arabic shaping is broken except for first word in line + https://bugs.webkit.org/show_bug.cgi?id=178625 + + Reviewed by Michael Catanzaro. + + Once we find the first space, which has the COMMON script, we split the run, and the next ones keep using + COMMON instead of ARABIC because we don't update the current script on every loop iteration. This patch + simplifies the script handling by moving the code back to the loop and always breaking in case of different + scripts, correctly handling INHERITED and COMMON cases and updating the current script when needed. + + Covered by existing tests. This improves several tests that have been rebaselined. + + * platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp: + (WebCore::findNextRun): + (WebCore::scriptsAreCompatibleForCharacters): Deleted. + 2017-10-17 Carlos Garcia Campos [GTK][Stable] BackingStoreBackendCairoImpl.h:23:10: fatal error: WebCore/HysteresisActivity.h diff --git a/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp b/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp index 5736903e1909..b256f7858b66 100644 --- a/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp +++ b/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp @@ -364,33 +364,6 @@ FloatPoint HarfBuzzShaper::adjustStartPoint(const FloatPoint& point) return point + m_startOffset; } -static bool scriptsAreCompatibleForCharacters(UScriptCode script, UScriptCode previousScript, UChar32 character, UChar32 previousCharacter) -{ - if (script == previousScript) - return true; - - if (script == USCRIPT_INHERITED || previousScript == USCRIPT_COMMON) - return true; - - if (script == USCRIPT_COMMON) { - // §5.1 Handling Characters with the Common Script Property. - // Programs must resolve any of the special Script property values, such as Common, - // based on the context of the surrounding characters. A simple heuristic uses the - // script of the preceding character, which works well in many cases. - // http://www.unicode.org/reports/tr24/#Common. - // - // FIXME: cover all other cases mentioned in the spec (ie. brackets or quotation marks). - // https://bugs.webkit.org/show_bug.cgi?id=177003. - // - // We use a slightly more conservative heuristic than the one proposed in the spec, - // using the script of the previous character only if both are ASCII. - if (isASCII(character) && isASCII(previousCharacter)) - return true; - } - - return uscript_hasScript(character, previousScript); -} - bool HarfBuzzShaper::collectHarfBuzzRuns() { const UChar* normalizedBufferEnd = m_normalizedBuffer.get() + m_normalizedBufferLength; @@ -413,7 +386,6 @@ bool HarfBuzzShaper::collectHarfBuzzRuns() if (!currentFontData) currentFontData = &m_font->primaryFont(); UScriptCode currentScript = nextScript; - UChar32 previousCharacter = character; for (iterator.advance(clusterLength); iterator.consume(character, clusterLength); iterator.advance(clusterLength)) { if (FontCascade::treatAsZeroWidthSpace(character)) @@ -447,14 +419,31 @@ bool HarfBuzzShaper::collectHarfBuzzRuns() if (nextFontData != currentFontData) break; - if (!scriptsAreCompatibleForCharacters(nextScript, currentScript, character, previousCharacter)) - break; + // §5.1 Handling Characters with the Common Script Property. + // Programs must resolve any of the special Script property values, such as Common, + // based on the context of the surrounding characters. A simple heuristic uses the + // script of the preceding character, which works well in many cases. + // http://www.unicode.org/reports/tr24/#Common. + // + // FIXME: cover all other cases mentioned in the spec (ie. brackets or quotation marks). + // https://bugs.webkit.org/show_bug.cgi?id=177003. + // + // If next script is inherited or common, keep using the current script. + if (nextScript == USCRIPT_INHERITED || nextScript == USCRIPT_COMMON) { + currentCharacterPosition = iterator.characters(); + continue; + } + // If current script is inherited or common, set the next script as current. + if (currentScript == USCRIPT_INHERITED || currentScript == USCRIPT_COMMON) { + currentScript = nextScript; + currentCharacterPosition = iterator.characters(); + continue; + } - if (nextScript == USCRIPT_INHERITED || nextScript == USCRIPT_COMMON) - nextScript = currentScript; + if (currentScript != nextScript && !uscript_hasScript(character, currentScript)) + break; currentCharacterPosition = iterator.characters(); - previousCharacter = character; } unsigned numCharactersOfCurrentRun = iterator.currentIndex() - startIndexOfCurrentRun; hb_script_t script = hb_icu_script_to_script(currentScript);