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

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Ojan Vafai <ojan at chromium.org>  2010-03-10 09:10:26 PST ---
(From update of attachment 50376)
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,27 @@
> +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' guranuality implies just
> +        "Go to the end of the line", unlike selection expansioin 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.
> +        This change also fixed getInlineBoxAndOffset() that didn't handle
> +        an edge case for for UPSTREAM.

Nit: you repeat this last line below. Only need to put it in one place. Either
place is fine.

> +
> +        Re-baseline: editing/selection/extend-selection-expected.txt
> +
> +        * dom/Position.cpp:
> +        (WebCore::Position::getInlineBoxAndOffset):
> +        Handled an edge case for node lookup with UPSTREAM.
> +        * editing/SelectionController.cpp:
> +        (WebCore::SelectionController::modifyExtendingForward):
> +        Added UPSTREAM tweak for the case for LineGranularity.

Nit: This line is redundant with the change description above.

I'm definitely nit-picking here, but you only need to annotate specific
files/methods if there's something extra worth noting. Or, you can leave the
change description minimal and annotate the files/methods instead.

The code changes seem reasonable to me. Does modifyMovingForward also need to
be fixed? What about sentenceboundary/paragraphboundary? I'm not really sure
what sentenceboundary/paragraphboundary are used for, i.e. do they correspond
to a user-action? If they do, it would be good to compare TextEdit and WebKit
to see if this needs to be done for those cases as well.

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