Bug 115885 – <label> element should send key and focus events if it has contenteditable attribute.

• Component: Forms
• Status: NEW

[Yael]

HTMLLabelElement should be focusable if it passes the test isContentEditable(). As a result, the proper events would be sent.
Both Opera and Firefox support this behavior.

Merge of https://src.chromium.org/viewvc/blink?view=rev&revision=149973

[Yael]

Created attachment 201311 [details]
Patch

[Benjamin Poulain]

Comment on attachment 201311 [details]
Patch

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()???

[Yael]

(In reply to comment #2)
> Shouldn't you instead assert there is no pending style recalc and use rendererIsEditable()???

Can you share how to decide when to assert and when to call updateStyleIfNeeded ?

[Ahmad Saleem]

Taking test from Chrome bug and then trying to type in the Label field, it does work and show "keyup" event in Safari 16.2:

Link - https://jsfiddle.net/faceleg/VBdDK/1/

Is this needed anymore? Thanks!