[Webkit-unassigned] [Bug 62092] setting innerText to an empty string on editable div loses focus
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jun 4 17:14:47 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=62092
Ryosuke Niwa <rniwa at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #96026|review? |review-
Flag| |
--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> 2011-06-04 17:14:47 PST ---
(From update of attachment 96026)
View in context: https://bugs.webkit.org/attachment.cgi?id=96026&action=review
I'd say r- because there are quite few nits.
> Source/WebCore/ChangeLog:7
> + After setting text to empty string and removing children nodes selection will be set to noselection.
> + If this is the currently focused element update the focus appearance which will restore the carret.
> + https://bugs.webkit.org/show_bug.cgi?id=62092
Missing bug title. a change log entry should be (note there's a blank line between bug url & description):
bug title
bug url
description
Also, typo: s/carret/caret/.
> Source/WebCore/html/HTMLElement.cpp:456
> + if (document()->focusedNode() == this)
> + updateFocusAppearance(true);
I'm not sure if this is the right fix. Removing child nodes should have invoked FrameSelection::nodeWillBeRemoved. Why do we need to call updateFocusAppearance here manually?
> LayoutTests/ChangeLog:7
> +
You should explain what kind of a test you're adding (blank lines before and after the description).
> LayoutTests/fast/dom/HTMLElement/editable-div-clear-on-keydown.html:1
> +<html>
Missing <!DOCTYPE html>.
> LayoutTests/fast/dom/HTMLElement/editable-div-clear-on-keydown.html:22
> + if (event.keyCode === 82) { //r
Is it significant that we check for this key code? I'd rather always run the test when we have a keydown event.
> LayoutTests/fast/dom/HTMLElement/editable-div-clear-on-keydown.html:31
> +<body onload="test()">
I don't see any reason we have to wait load event. You should just put script in body so that you don't have to wait for load event.
> LayoutTests/fast/dom/HTMLElement/editable-div-clear-on-keydown.html:33
> +<p>The test runs only under DumpRenderTree with eventSender; if you test by hand the test result below will say FAIL.</p>
It seems like this test can be ran manually by pressing some key down in the editable region below, and observing that the character is inserted.
> LayoutTests/fast/dom/HTMLElement/editable-div-clear-on-keydown.html:34
> +<div id="it" style="-webkit-user-modify: read-write;-webkit-user-select: text;cursor: text;" onKeyDown="resetIt(event)">text</div>
s/onKeyDown/onkeydown/. Also, are these properties all necessary for reproduction (especially -webkit-user-select: text and cursor: text;)?
--
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