[Webkit-unassigned] [Bug 115885] <label> element should send key and focus events if it has contenteditable attribute.

[bugzilla-daemon at webkit.org]

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


Benjamin Poulain <benjamin at webkit.org> changed:

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




--- Comment #2 from Benjamin Poulain <benjamin at webkit.org>  2013-05-09 22:43:49 PST ---
(From update of attachment 201311)
View in context: https://bugs.webkit.org/attachment.cgi?id=201311&action=review

> LayoutTests/fast/forms/label/labels-contenteditable.html:16
> +var parent = document.createElement('div');
> +
> +parent.innerHTML = '<label id="label1" for="input1">text</label><input id="input1"> <label id="label2" contenteditable for="input2">text</label><input id="input2">';

Why do you do that?
Why isn't that in the markup?

> LayoutTests/fast/forms/label/labels-contenteditable.html:34
> +label1.addEventListener('keydown', function () { alert("FAIL: keydown should not be sent if the label does not have contenteditable attribute"); });
> +label1.addEventListener('keypress', function () { alert("FAIL: keypress should not be sent if the label does not have contenteditable attribute"); });
> +label1.addEventListener('keyup', function () { alert("FAIL: keyup should not be sent if the label does not have contenteditable attribute"); });
> +label1.addEventListener('focus', function () { alert("FAIL: focus should not be sent if the label does not have contenteditable attribute"); });
> +label1.addEventListener('blur', function () { alert("FAIL: blur should not be sent if the label does not have contenteditable attribute"); });
> +input1.addEventListener('focus', function () { alert("PASS: label passed the focus to the corresponding control"); });
> +label2.addEventListener('keydown', function () { alert("PASS: keydown should be sent if the label has contenteditable attribute"); });
> +label2.addEventListener('keypress', function () { alert("PASS: keypress should be sent if the label has contenteditable attribute"); });
> +label2.addEventListener('keyup', function () { alert("PASS: keyup should be sent if the label has contenteditable attribute"); });
> +label2.addEventListener('focus', function () { alert("PASS: focus should be sent if the label has contenteditable attribute"); });
> +label2.addEventListener('blur', function () { alert("PASS: blur should be sent if the label has contenteditable attribute"); });
> +input2.addEventListener('focus', function () { alert("PASS: label passed the focus to the corresponding control"); });

You should use debug() logging instead of alert().

> LayoutTests/fast/forms/resources/label-test-util.js:80
> +function mouseMoveToLabel(labelId) {
> +    var label = document.getElementById(labelId);
> +    var itemHeight = Math.floor(label.offsetHeight / label.size);
> +    var offset = 5;
> +    if (window.eventSender)
> +        eventSender.mouseMoveTo(label.offsetLeft + offset, label.offsetTop + offset - window.pageYOffset);
> +}

There is already code to do that in LayoutTest. None that you can use?

> Source/WebCore/html/HTMLLabelElement.cpp:69
> -    return false;
> +    HTMLLabelElement* that = const_cast<HTMLLabelElement*>(this);
> +    return that->isContentEditable();

This is very much non-const.

Shouldn't you instead assert there is no pending style recalc and use rendererIsEditable()???

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