Bug 51851 - Implement RenderSVGInlineText::localCaretRect()
Summary: Implement RenderSVGInlineText::localCaretRect()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Justin Garcia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-03 15:56 PST by Justin Garcia
Modified: 2011-01-09 21:48 PST (History)
4 users (show)

See Also:


Attachments
patch (4.29 KB, patch)
2011-01-07 16:09 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch (4.62 KB, patch)
2011-01-07 17:24 PST, Justin Garcia
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Garcia 2011-01-03 15:56:17 PST
RenderSVGInlineText::localCaretRect() currently just returns an empty rect.
Comment 1 Justin Garcia 2011-01-07 16:09:34 PST
Created attachment 78287 [details]
patch
Comment 2 mitz 2011-01-07 16:18:55 PST
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.
Comment 3 Justin Garcia 2011-01-07 17:24:29 PST
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.
Comment 4 Justin Garcia 2011-01-07 19:01:32 PST
Commited in r75308
Comment 5 Antonio Gomes 2011-01-08 09:06:14 PST
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.
Comment 6 Justin Garcia 2011-01-09 21:48:28 PST
(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.