[Webkit-unassigned] [Bug 49511] RTL: Caret goes to the opposite direction when pressing an arrow key after selection is made

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Ryosuke Niwa <rniwa at webkit.org>  2010-12-08 16:28:48 PST ---
(From update of attachment 75041)
View in context: https://bugs.webkit.org/attachment.cgi?id=75041&action=review

> WebCore/editing/SelectionController.cpp:285
> -            m_selection.setBase(start);
> -            m_selection.setExtent(end);
> +            if (directionOfEnclosingBlock() == LTR) {
> +                m_selection.setBase(start);
> +                m_selection.setExtent(end);
> +            } else {
> +                m_selection.setBase(end);
> +                m_selection.setExtent(start);
> +            }

I don't think we should be doing this for DirectionRight.  New code seems to make sense only for DirectionForward.

Dan & Xiaomei, could you comment on this?

> WebCore/editing/SelectionController.cpp:295
> -            m_selection.setBase(end);
> -            m_selection.setExtent(start);
> +            if (directionOfEnclosingBlock() == LTR) {
> +                m_selection.setBase(end);
> +                m_selection.setExtent(start);
> +            } else {
> +                m_selection.setBase(start);
> +                m_selection.setExtent(end);
> +            }

Ditto.

> WebCore/editing/SelectionController.cpp:425
> -            pos = VisiblePosition(m_selection.end(), m_selection.affinity());
> +            if (directionOfEnclosingBlock() == LTR)
> +                pos = VisiblePosition(m_selection.end(), m_selection.affinity());
> +            else
> +                pos = VisiblePosition(m_selection.start(), m_selection.affinity());

You could write this as:
pos = VisiblePosition(directionOfEnclosingBlock() == LTR ? m_selection.end() : m_selection.start(), m_selection.affinity());
But maybe this is less readable.

> LayoutTests/editing/selection/rtl-move-selection-right-left.html:71
> +    testSelectionChange(setSelection, "move", "right", 0, undefined);
> +    testSelectionChange(setSelection, "move", "left", undefined, 0);

Why don't we test end & start offsets for these two tests?  I think we should.

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