[Webkit-unassigned] [Bug 91912] Adding "all" to -webkit-user-select
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 23 23:43:47 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=91912
--- Comment #65 from Ryosuke Niwa (vacation: 8/26-9/10) <rniwa at webkit.org> 2012-08-23 23:43:44 PST ---
(From update of attachment 159961)
View in context: https://bugs.webkit.org/attachment.cgi?id=159961&action=review
Sorry more nits.
> Source/WebCore/page/EventHandler.cpp:830
> + // reset base for user select all when base is inside user-select-all area and extent < base.
Nit: Capitalize R.
> LayoutTests/editing/selection/user-select-all-selection-expected.txt:8
> +window.getSelection().modify('extend', 'forward', 'character')
> +PASS Selection is the entire user-select-all element
This looks much better!
> LayoutTests/editing/selection/user-select-all-selection.html:48
> +
Please remove leading whitespace and ditto for other lines.
> LayoutTests/editing/selection/user-select-all-selection.html:57
> + execSetSelectionCommand(userSelectAllElement.previousSibling.firstChild, userSelectAllElement.previousSibling.textContent.length, userSelectAllElement.previousSibling.firstChild, userSelectAllElement.previousSibling.textContent.length);
Maybe you can create a helper function like placeCaretInsideUserSelectAllElement and call it inside evalAndLog as in:
evalAndLog("placeCaretInsideUserSelectAllElement(); getSelection().modify('extend', 'forward', 'character')");
> LayoutTests/editing/selection/user-select-all-selection.html:61
> + evalAndLog("window.getSelection().modify('extend', 'backward', 'character')");
Shouldn't we reset the selection using execSetSelectionCommand?
> LayoutTests/editing/selection/user-select-all-selection.html:83
> + // test click
This comment is probably not that helpful.
> LayoutTests/editing/selection/user-select-all-selection.html:87
> + clickAt(clickTarget.offsetLeft + 10 , clickTarget.offsetTop + 10);
Maybe we can make this function take an element instead of x & y so that you can do:
evalAndLog("click(document.getElementById('elementInsideUserSelectAll'));");
> LayoutTests/editing/selection/user-select-all-selection.html:88
> + log("clicking on descendent of user-select-all element");
Then you wouldn't need this description.
> LayoutTests/editing/selection/user-select-all-selection.html:95
> + mouseMoveFromTo(leftTarget.offsetLeft, leftTarget.offsetTop + 10, clickTarget.offsetLeft + 20 , clickTarget.offsetTop + 10);
> + log("mouse extending from left to user-select-all element");
Here, you can pass in two x coordinates as in:
evalAndLog("moveMouseInElement(userSelectAllElement.offsetTop + 10, userSelectAllElement.previousSibling.offsetLeft, document.getElementById('elementInsideUserSelectAll').offsetLeft + 10)");
> LayoutTests/editing/selection/user-select-all-selection.html:102
> + var rightTargetTextLength = rightTarget.firstChild.textContent.length;
> + mouseMoveFromTo(userSelectAllElement.offsetLeft + userSelectAllElement.offsetWidth +rightTarget.offsetWidth, rightTarget.offsetTop + 10, clickTarget.offsetLeft + 10, clickTarget.offsetTop + 10);
Ditto.
> LayoutTests/editing/selection/user-select-all-selection.html:108
> +<body contentEditable="true" id = "body"><font>Test -webkit-user-select all</font><font class="userSelectAll" id ="allArea"><font id = "descendent">user select all area</font>user select all area</font><font>Test -webkit-user-select all</font>
Please be consistent about spaces around =. In most tests, we omit spaces around =.
> LayoutTests/editing/selection/user-select-all-selection.html:114
> + <script>
> + description(" Test -webkit-user-select all selection movements and extensions (left right forward backward) ");
> + testKeyboard();
> + testMouse();
> + </script>
Why do we need to put this in a separate script element?
--
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