[webkit-reviews] review granted: [Bug 34865] REGRESSION (r50301): Problem selecting text in a Devanagari website : [Attachment 48598] Account for non-monotonic character-to-glyph mapping

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted mitz at webkit.org's
request for review:
Bug 34865: REGRESSION (r50301): Problem selecting text in a Devanagari website
https://bugs.webkit.org/show_bug.cgi?id=34865

Attachment 48598: Account for non-monotonic character-to-glyph mapping
https://bugs.webkit.org/attachment.cgi?id=48598&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================
> +	   * platform/graphics/mac/ComplexTextController.cpp:
> +	   (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
Initialize m_isMonotonic.
> +	   (WebCore::ComplexTextController::ComplexTextRun::setIsMonotonic):
Added. Sets m_isMonotonic,
> +	   and if the run is not monotonic, populates m_lastInidices with the
end offsets of each glyph’s

A typo and a garbled curly quote here (and one more lower down).

> Index: WebCore/platform/graphics/mac/ComplexTextController.cpp
> ===================================================================

> +void ComplexTextController::ComplexTextRun::setIsMonotonic(bool isMonotonic)

> +{
> +    m_isMonotonic = isMonotonic;
> +    if (isMonotonic)
> +	   return;

Why doesn't this do the usual (if )isMonotonic == m_isMonotonic) return;?

Also, if changing from non-monotonic to monotonic, should you clear
m_lastIndices?

> Index: WebCore/platform/graphics/mac/ComplexTextController.h
> ===================================================================

> +	   Vector<CFIndex, 64> m_lastIndices;

The name of this member doesn't immediately communicate what it stores. Maybe
m_glyphEndOffsets or something?

r=me