[Webkit-unassigned] [Bug 33413] selection.modify extends too far with 'lineboundary'

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 11 10:13:18 PST 2010


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





--- Comment #15 from Enrica Casucci <enrica at apple.com>  2010-03-11 10:13:18 PST ---
(From update of attachment 50470)
> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index 9d47289..d30ce63 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,15 @@
> +2010-03-10  MORITA Hajime  <morrita at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Selection.modify extends too far with 'lineboundary'.
> +        https://bugs.webkit.org/show_bug.cgi?id=33413
> +
> +        * editing/selection/extend-selection-expected.txt:
> +        Updated result to correct expectation that described the wrong
> +        behaviour for the selection expansion with 'lineboundary'
> +        granularity.
> +
>  2010-03-10  Roland Steiner  <rolandsteiner at chromium.org>
>  
>          Reviewed by David Levin.
> diff --git a/LayoutTests/editing/selection/extend-selection-expected.txt b/LayoutTests/editing/selection/extend-selection-expected.txt
> index 1617e03..0b33cd2 100644
> --- a/LayoutTests/editing/selection/extend-selection-expected.txt
> +++ b/LayoutTests/editing/selection/extend-selection-expected.txt
> @@ -514,14 +514,14 @@ Test 20, RTL:
>    Extending forward: "ABC abc DEF"[(0,0), (0,11)]
>    Extending backward:  "ABC abc DEF"[(0,11), (0,0)]
>  Test 21, LTR:
> -  Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,8), (0,16), (0,26), (0,34), (0,42), (0,50), (0,58), (0,66), (0,74), (0,82), (0,90), (0,98), (0,105)]
> -  Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,105), (0,0)]
> +  Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,8)]
> +  Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,8), (0,0)]
>  Test 21, RTL:
>    Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,7)]
>    Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,7), (0,0)]
>  Test 22, LTR:
> -  Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,8), (0,16), (0,26), (0,34), (0,42), (0,50), (0,58), (0,66), (0,74), (0,82), (0,90), (0,98), (0,105)]
> -  Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,105), (0,0)]
> +  Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,8)]
> +  Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,8), (0,0)]
>  Test 22, RTL:
>    Extending forward: "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,0), (0,7)]
>    Extending backward:  "abcdefg abcdefg abcdefg a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg "[(0,7), (0,0)]
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 0a812be..afda4f5 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,25 @@
> +2010-03-10  MORITA Hajime  <morrita at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Selection.modify extends too far with 'lineboundary'.
> +        https://bugs.webkit.org/show_bug.cgi?id=33413
> +
> +        Selection.modify() with 'lineboundary' granularity implies just
> +        "Go to the end of the line", unlike selection expansion with
> +        other type of granularities. This change handled LineGranularity
> +        as special case, to look-up end of line with UPSTREAM affinity. 
> +        Doing this prevents look-up algorithm to go next line.
> +
> +        Test: editing/selection/extend-selection-expected.txt
> +
> +        * dom/Position.cpp:
> +        (WebCore::Position::getInlineBoxAndOffset):
> +        Handled an edge case for node look-up with UPSTREAM.
> +        * editing/SelectionController.cpp:
> +        (WebCore::SelectionController::modifyExtendingForward):
> +        Added UPSTREAM tweak for the case for LineGranularity.
> +        
>  2010-03-10  Roland Steiner  <rolandsteiner at chromium.org>
>  
>          Unreviewed build fix. Fix variable name change that somehow didn't
> diff --git a/WebCore/dom/Position.cpp b/WebCore/dom/Position.cpp
> index ef0b3c1..e22dc9e 100644
> --- a/WebCore/dom/Position.cpp
> +++ b/WebCore/dom/Position.cpp
> @@ -1041,6 +1041,9 @@ void Position::getInlineBoxAndOffset(EAffinity affinity, TextDirection primaryDi
>                  return;
>              }
>  
> +            if ((caretOffset == caretMaxOffset) ^ (affinity == DOWNSTREAM))
> +                break;
> +

Why this? affinity is only UPSTREAM or DOWNSTREAM.
You could change the code to be
             if (caretOffset == caretMinOffset)
                  break;
and have only one if statement instead of 2.

>              if ((caretOffset == caretMinOffset) ^ (affinity == UPSTREAM))
>                  break;
>  
> diff --git a/WebCore/editing/SelectionController.cpp b/WebCore/editing/SelectionController.cpp
> index c0f03de..f2859fd 100644
> --- a/WebCore/editing/SelectionController.cpp
> +++ b/WebCore/editing/SelectionController.cpp
> @@ -351,7 +351,14 @@ VisiblePosition SelectionController::modifyExtendingForward(TextGranularity gran
>              pos = endOfSentence(endForPlatform());
>              break;
>          case LineBoundary:
> -            pos = logicalEndOfLine(endForPlatform());
> +            // LineBoundary granularity actually emulates
> +            // NSTextView#moveToRightEndOfLineAndModifySelection(),
> +            // which never expands a selection across multiple lines.
> +            // So we set UPSTREAM affinity to prevent crossing the
> +            // line boundary when the offset is at the end of the line.
> +            pos = endForPlatform();
> +            pos.setAffinity(UPSTREAM);
> +            pos = logicalEndOfLine(pos);
>              break;
>          case ParagraphBoundary:
>              pos = endOfParagraph(endForPlatform());

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



More information about the webkit-unassigned mailing list