Bug 51851 – Implement RenderSVGInlineText::localCaretRect()

• Component: SVG
• Status: RESOLVED FIXED

[Justin Garcia]

RenderSVGInlineText::localCaretRect() currently just returns an empty rect.

[Justin Garcia]

Created attachment 78287 [details]
patch

Comment on attachment 78287 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78287&action=review

> WebCore/rendering/svg/RenderSVGInlineText.cpp:94
>  

Please use static_cast<> everywhere you used C-style casts below.

> WebCore/rendering/svg/RenderSVGInlineText.cpp:107
> +        return IntRect(rect.x(), rect.y(), caretWidth, rect.height());

If this is a right-to-left box, do we really want the left edge of the range?

> WebCore/rendering/svg/RenderSVGInlineText.cpp:111
> +    return IntRect(rect.x() + rect.width() - caretWidth, rect.y(), caretWidth, rect.height());

rect.x() + rect.width() can be written as rect.right(). If this a right-to-left box, do we really want the right edge of the range?

> LayoutTests/svg/text/text-style-invalid-expected.txt:1
> +This test passes if we do NOT crash while rendering SVG text with CSS styles :first-letter and :first-line. 

Is this change related? Seems not.

[Justin Garcia]

Created attachment 78293 [details]
patch

Updated patch.  Added testing for RTL text, although currently at the edge of RTL runs a bug in Editor::firstRectForRange(Range* range) passes the wrong offset to RenderSVGInline::localCaretRect.  The test illustrates these expected failures.

[Justin Garcia]

Commited in r75308

[Antonio Gomes]

http://build.webkit.org/builders/Qt%20Linux%20Release/builds/26202/steps/layout-test/logs/stdio

svg/text/caret-in-svg-text.xhtml -> failed


Generally the committer skips test or investigates the failure. Also the case of the former, a bug is also filed.

[Justin Garcia]

(In reply to comment #5)
> http://build.webkit.org/builders/Qt%20Linux%20Release/builds/26202/steps/layout-test/logs/stdio
> 
> svg/text/caret-in-svg-text.xhtml -> failed

firstRectForCharacterRange is unimplemented on qt.  Added the test to the Skipped list in r75360.