[Webkit-unassigned] [Bug 48037] Triple click does not select whole line for mixed contenteditable regions

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Ryosuke Niwa <rniwa at webkit.org>  2010-10-25 21:50:07 PST ---
(From update of attachment 71839)
View in context: https://bugs.webkit.org/attachment.cgi?id=71839&action=review

> LayoutTests/editing/selection/triple-click-across-editability-boundaries.html:37
> +    layoutTestController.dumpAsText();
> +    var testElementIds = ["test1Element", "test2Element", "test3Element"];
> +    for (var i in testElementIds) {
> +        var element = document.getElementById(testElementIds[i]);
> +        log("Results for " + element.id + ":\n");
> +        tripleClickAndLog(element, "left");
> +        tripleClickAndLog(element, "middle");
> +        tripleClickAndLog(element, "right");
> +    }

You probably want to rewrite this as a script-test.  See http://trac.webkit.org/changeset/68670 for an example.

> LayoutTests/editing/selection/triple-click-across-editability-boundaries.html:58
> +    eventSender.mouseDown();
> +    eventSender.mouseUp();
> +    eventSender.mouseDown();
> +    eventSender.mouseUp();
> +    eventSender.mouseDown();
> +    eventSender.mouseUp();

This might be too fast.  You may need to add some delays between each downs & ups.

> WebCore/editing/visible_units.cpp:817
> +        // FIXME: We avoid returning a position where the renderer can't accept the caret.
> +        // We also do this in endOfParagraph, and might want to do it in other cases too.

Why FIXME?  Isn't new code correct?  I don't think we should add FIXME to indicate that we should fix other places.  And I note that there are other places in editing where we skip empty text nodes.  If you've found other code that may require skipping empty text nodes, then you should add FIXME to those places and not here.

> WebCore/editing/visible_units.cpp:-877
>          // FIXME: We avoid returning a position where the renderer can't accept the caret.
> -        // We should probably do this in other cases such as startOfParagraph.

You should get rid of this FIXME entirely since we now do the same in startOfParagraph.

> WebCore/editing/visible_units.h:37
> +enum EditingBoundaryCrossingRule { CanCrossEditingBoundary, CannotCrossEditingBoundary };

Mn... you probably don't want to redefine enum here.

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