[Webkit-unassigned] [Bug 45889] Style visibility: hidden on <br/> tags causes input fields to lose focus after deleting all content

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #80600|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #13 from Ryosuke Niwa <rniwa at webkit.org>  2011-01-30 20:26:15 PST ---
(From update of attachment 80600)
View in context: https://bugs.webkit.org/attachment.cgi?id=80600&action=review

> Source/WebCore/ChangeLog:8
> +

Please explain what caused the bug and how you fixed it.

> Source/WebCore/editing/DeleteSelectionCommand.cpp:800
> +    // For text input elements, BreakElement place holder is not needed.
> +    // Because any BR styles should not affect text field properties. 

This comment is redundant it repeats what code says.

> LayoutTests/ChangeLog:8
> +

Please explain what kind of test you're adding.

> LayoutTests/editing/deleting/textfield-loose-focus-with-br.html:16
> +<script>
> +if (window.layoutTestController)
> +     layoutTestController.dumpEditingCallbacks();
> +</script>
> +
> +<script>
> +if (window.layoutTestController) {
> +    layoutTestController.waitUntilDone();
> +    layoutTestController.dumpAsText();
> +}
> +</script>

Please combine these two script elements.  And also, I don't think you need to call dumpEditingCallbacks in this test unless there are some delegate callbacks you want to test.

Also, why are you calling waitUntilDone() here?  waitUntilDone() is called when the test needs to continue to run after page load event, and I don't see any reason we want such a behavior in this test.

> LayoutTests/editing/deleting/textfield-loose-focus-with-br.html:22
> +<div><br></div>

What is this div & br doing here?

> LayoutTests/editing/deleting/textfield-loose-focus-with-br.html:38
> +if(text === "XYZ")
> +	document.write("<div> test SUCCESS <\div>");
> +else
> +	document.write("<div> test FAILED <\div>");

Better written as:
document.write(text == "XYZ" ? "PASS" : "FAIL");

> LayoutTests/editing/deleting/textfield-loose-focus-with-br.html:45
> +<script>
> +if (window.layoutTestController) {
> +    layoutTestController.notifyDone()
> +}
> +</script>

If we stop calling waitUntilDone(), then this entire script can go away.

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