[webkit-reviews] review denied: [Bug 127324] REGRESSION (r160259?): text-combine glyphs are not rendered : [Attachment 224467] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied  review:
Bug 127324: REGRESSION (r160259?): text-combine glyphs are not rendered
https://bugs.webkit.org/show_bug.cgi?id=127324

Attachment 224467: Patch
https://bugs.webkit.org/attachment.cgi?id=224467&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=224467&action=review


I think at minimum this needs to have a good test case.

> Source/WebCore/ChangeLog:16
> +	   No new tests.
> +	   Image test of fast/text/international/text-combine-image-test.html
covers this.

It shouldn't be hard to add a proper test for this that doesn't require running
in pixel mode. Reftest (that is test.html/test-expected.html pair) might be
easiest.

> Source/WebCore/rendering/svg/RenderSVGInlineText.h:55
> +    virtual void setTextInternal(const String&, bool replace = false)
override;

It would be better to add an explicit API,
RenderText::overrideOriginalTextForTextCombine or similar instead of adding a
confusing new parameter. (Also I don't fully understand why RenderCombineText
needs this special behavior.)