[Webkit-unassigned] [Bug 105451] Unable to place caret in RTL Override Characters by mouse click

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105451





--- Comment #12 from Ryosuke Niwa <rniwa at webkit.org>  2013-04-10 14:44:05 PST ---
(From update of attachment 197354)
View in context: https://bugs.webkit.org/attachment.cgi?id=197354&action=review

> LayoutTests/editing/selection/click-on-rtl-text-in-textarea-expected.txt:8
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS

This output isn't helpful. It doesn't tell us what kind of test cases we have and why we're passing.

> LayoutTests/editing/selection/click-on-rtl-text-in-textarea.html:44
> +    clickOnText(editor.offsetLeft + 1, editor.offsetTop + 5);
> +    if (editor.selectionStart != 0)
> +        result = "FAIL";

You can replace this code by shouldBe("clickOnText(editor.offsetLeft + 1, editor.offsetTop + 5); editor.selectionStart", "0");
after including js-test-pre.js. This way, we can see what we're testing and what we're checking.
It would better if we included element id names so that we can distinguish each test case.

> LayoutTests/editing/selection/click-on-rtl-text-in-textarea.html:48
> +    clickOnText(editor.offsetLeft + 30, editor.offsetTop + 5);
> +    if (editor.selectionStart <= 1)
> +        result = "FAIL";

Ditto.

> Source/WebCore/platform/text/BidiResolver.h:149
> +    bool onlyHasNewlineCharacter() const { return false; }

Why is it okay to always return false here?

> Source/WebCore/platform/text/BidiResolver.h:508
> +    // According to the Bidi spec L1, reset the embedding level of the
> +    // line break character to the paragraph embedding level.

Why do we need to do that? We need "why" comments, not "what" comments.

> Source/WebCore/rendering/BidiRun.cpp:78
> +bool BidiRun::onlyHasNewlineCharacter() const
> +{
> +    return (m_stop - m_start == 1) && m_object->isText() && toRenderText(m_object)->characterAt(m_start) == '\n';
> +}

This should definitely be an inline function.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.