[webkit-reviews] review denied: [Bug 36840] SVG @font-face breaks text-overflow: ellipsis : [Attachment 80598] Patch (2nd rev)

Mon Feb 7 11:27:29 PST 2011

Dirk Schulze <krit at webkit.org> has denied Emil Schutte
<emilschutte at gmail.com>'s request for review:
Bug 36840: SVG @font-face breaks text-overflow: ellipsis
https://bugs.webkit.org/show_bug.cgi?id=36840

Attachment 80598: Patch (2nd rev)
https://bugs.webkit.org/attachment.cgi?id=80598&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80598&action=review

Nearly done:

> Source/WebCore/ChangeLog:18
> +	   Implement offsetForPositionForTextUsingSVGFont for SVG fonts, fixing

> +	   rendering of SVG font-styled text with text-overflow: ellipsis and
> +	   per-character selection of SVG font-styled text.  Uses existing
> +	   SVGTextRunWalker mechanism to walk the text run, measuring total
pixel
> +	   length up to the specified limit, then returning the number of
characters
> +	   walked.
> +
> +	   SVG @font-face breaks text-overflow: ellipsis
> +	   https://bugs.webkit.org/show_bug.cgi?id=36840
> +	   REGRESSION: SVG Font selection problems
> +	   https://bugs.webkit.org/show_bug.cgi?id=41934
> +
> +	   Tests: editing/selection/select-text-svgfont.html
> +		  fast/css/text-overflow-ellipsis-svgfont.html

Please use this style:

bug name
bug link

bug name
bug link

comment

Tests:

> Source/WebCore/svg/SVGFont.cpp:609
> +    return (data.at < data.to && data.length <= data.maxLength);

No braces here

> Source/WebCore/svg/SVGFont.cpp:640
> +    int len = run.length();

name it length

> Source/WebCore/svg/SVGFont.cpp:658
> +    if (!(includePartialGlyphs && (data.maxLength > data.length -
(data.lastGlyphLength / 2))))

if (!includePartialGlyphs || !(data.maxLength > data.length -
(data.lastGlyphLength / 2)))

two braces less

> LayoutTests/fast/css/text-overflow-ellipsis-svgfont.html:1
> +<META HTTP-EQUIV="CONTENT-TYPE" CONTENT="TEXT/HTML; CHARSET=utf8">

<!DOCTYPE html>
<html>