[Webkit-unassigned] [Bug 25195] mouse-based selections are always directional on Window/Linux

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51110|review?                     |review-
               Flag|                            |




--- Comment #14 from David Levin <levin at chromium.org>  2010-03-24 16:05:49 PST ---
(From update of attachment 51110)
Nothing huge but some changes may be significant so r- for one more round.


> diff --git a/LayoutTests/editing/selection/5195166-1.html b/LayoutTests/editing/selection/5195166-1.html

Since you're changing the image result, would you consider making this a
dumpAsText test? (I think it already tests everything needed without any
changes.)

The plus side is that you won't have to generate the mac result which is
missing (but must be incorrect after this change.)


> diff --git a/LayoutTests/platform/win/editing/selection/extend-after-mouse-selection-expected.txt b/LayoutTests/platform/win/editing/selection/extend-after-mouse-selection-expected.txt

> +PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text](ef) focusOffset: 1 isCollapsed: false]
> +PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text](ef) focusOffset: 2 isCollapsed: false]
> +PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text]( ) focusOffset: 1 isCollapsed: false]
> +PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object HTMLElement](null) focusOffset: 4 isCollapsed: false]

From
http://trac.webkit.org/browser/trunk/LayoutTests/editing/selection/extend-after-mouse-selection.html
  " // Seleciton goes from the end of "ef" to the first BR element in the
contentEditable region."

Is the text of the test incorrect because it refers to "ef"? (If you change
this line, please fix this typo "Seleciton" as well.)


   "assertSelectionAt(endTarget.firstChild, 2, document.getElementById('test'),
3);"

I also don't understand how the assert in the test passes.


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2010-03-18  Ojan Vafai  <ojan at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        mouse-based selections are always directional on Windows/Linux
> +        https://bugs.webkit.org/show_bug.cgi?id=25195
> +
> +        Change m_lastChangeWasHorizontalExtension to m_isDirectional
> +        and make m_isDirectional always be true for Windows/Linux.
> +
> +        * editing/SelectionController.cpp:
> +        (WebCore::SelectionController::SelectionController):
> +        (WebCore::SelectionController::setSelection):
> +        (WebCore::SelectionController::setIsDirectional):
> +        (WebCore::SelectionController::willBeModified):
> +        When double-clicking the base/extent will be in the middle

When double-clicking,
(Added ,)

> +        of the seleciton instead of the start/end of it. Changed to

typo: seleciton

> +void SelectionController::setIsDirectional(bool isDirectional)
> +{
> +    Settings* settings = m_frame ? m_frame->settings() : 0;
> +    if (settings && settings->editingBehavior() == EditingMacBehavior)
> +        m_isDirectional = isDirectional;
> +    else
> +        m_isDirectional = true;

How do you feel about this?
        m_isDirectional = !settings || (settings->editingBehavior() ==
EditingMacBehavior && isDirectional);

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