[Webkit-unassigned] [Bug 91912] Adding "all" to -webkit-user-select

[bugzilla-daemon at webkit.org]

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





--- Comment #29 from Ryosuke Niwa <rniwa at webkit.org>  2012-08-16 14:43:43 PST ---
(From update of attachment 158896)
View in context: https://bugs.webkit.org/attachment.cgi?id=158896&action=review

> Source/WebCore/dom/Position.cpp:874
> +    return Position::nodeIsUserSelectAll(node) && node->parentNode() && !Position::nodeIsUserSelectAll(node->parentNode());

No need to qualify nodeIsUserSelectAll.
Also, we need a test for this code.

> Source/WebCore/dom/Position.cpp:886
> +    while (parent && Position::nodeIsUserSelectAll(parent)) {

Ditto about Position:: and needing a test.

> Source/WebCore/page/EventHandler.cpp:442
>          updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);

Why don't we call this in updateSelectionForMouseDownDispatchingSelectStart instead of repeating it everywhere?

> LayoutTests/editing/selection/user-select-all-selection-expected.txt:1
> +Test -webkit-user-select alllluser select all area

Nit: alllluser.

> LayoutTests/editing/selection/user-select-all-selection-expected.txt:8
> +PASS Selection is [anchorNode: [object HTMLBodyElement](null) anchorOffset: 1 focusNode: [object HTMLBodyElement](null) focusOffset: 2 isCollapsed: false]

This output doesn't really tell us what we're testing and why it's passing.

> LayoutTests/editing/selection/user-select-all-selection.html:13
> +                execSetSelectionCommand(body.firstChild, 30, body.firstChild, 30 );
> +                execExtendSelectionForwardByCharacterCommand();

You can put this in evalAndLog so that people can see what you're doing in the expected result.

> LayoutTests/editing/selection/user-select-all-selection.html:58
> +                eventSender.mouseMoveTo(clickTarget.offsetLeft + 200, clickTarget.offsetTop + 10);
> +                eventSender.mouseUp();
> +                assertSelectionAt(body, 1, body.firstChild.nextSibling.nextSibling, 13);

This test depends on font metrics. You need to use Ahem font and specify the font size by pixel or otherwise it'll fail on non-Mac ports.
Alternatively, you can figure out the appropriate width dynamically.

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