Bug 56271 – focus() call inside input event handler is ignored

• Component: HTML Editing
• Status: RESOLVED CONFIGURATION CHANGED

[Ryosuke Niwa]

When an input element is created and focus() is called on that element inside an input event handler of another input element, the focus stays on the input element from which the input event was fired and focus() call is effectively ignored.

http://crbug.com/75863

[Ryosuke Niwa]

Created attachment 85623 [details]
demo

Here's a reduction attached on the Chromium bug.  To reproduce the bug, type in any character into the input element.  The focus be moved to the new input element when new one appears but it doesn't on ToT WebKit.

[Tony Chang]

Isn't this because of http://trac.webkit.org/changeset/58829 ?  There was some additional discussion on bug 39791.

[Ryosuke Niwa]

(In reply to comment #2)
> Isn't this because of http://trac.webkit.org/changeset/58829 ?  There was some additional discussion on bug 39791.

As far as I read the change, it only affects the case where the event handler sets the focus on a different frame.

[Levi Weintraub]

Here is the backtrace for the call that blows away the javascript focus() call back to the original input.

#0	0x10192f249 in WebCore::FocusController::setFocusedNode at FocusController.cpp:353
#1	0x1021521ca in WebCore::SelectionController::setFocusedNodeIfNeeded at SelectionController.cpp:1686
#2	0x102156e28 in WebCore::SelectionController::setSelection at SelectionController.cpp:182
#3	0x1018cd70d in WebCore::Editor::changeSelectionAfterCommand at Editor.cpp:3077
#4	0x1018d7f72 in WebCore::Editor::appliedEditing at Editor.cpp:1071
#5	0x1022adc07 in WebCore::TypingCommand::typingAddedToOpenCommand at TypingCommand.cpp:349
#6	0x1022af405 in WebCore::TypingCommand::insertTextRunWithoutNewlines at TypingCommand.cpp:401
#7	0x1022af4ba in WebCore::TypingCommand::insertText at TypingCommand.cpp:375
#8	0x1022af668 in WebCore::TypingCommand::doApply at TypingCommand.cpp:299
#9	0x1018c4aa7 in WebCore::EditCommand::apply at EditCommand.cpp:92
#10	0x1018c4b44 in WebCore::applyCommand at EditCommand.cpp:224
#11	0x1022b039e in WebCore::TypingCommand::insertText at TypingCommand.cpp:200
#12	0x1018d6bb8 in WebCore::Editor::insertTextWithoutSendingTextEvent at Editor.cpp:1198
#13	0x1018d6d8c in WebCore::Editor::handleTextEvent at Editor.cpp:208
#14	0x1018ef3fc in WebCore::EventHandler::defaultTextInputEventHandler at EventHandler.cpp:2762
#15	0x101ee4597 in WebCore::Node::defaultEventHandler at Node.cpp:3033
#16	0x101a56370 in WebCore::HTMLFormControlElementWithState::defaultEventHandler at HTMLFormControlElement.cpp:538
#17	0x101a6b8b9 in WebCore::HTMLInputElement::defaultEventHandler at HTMLInputElement.cpp:1074
#18	0x101ee4e94 in WebCore::Node::dispatchGenericEvent at Node.cpp:2727
#19	0x101ee5139 in WebCore::Node::dispatchEvent at Node.cpp:2625
#20	0x1019069f6 in WebCore::EventTarget::dispatchEvent at EventTarget.cpp:297
#21	0x1018f4ed8 in WebCore::EventHandler::handleTextInputEvent at EventHandler.cpp:2727
#22	0x1018d1739 in WebCore::Editor::insertText at Editor.cpp:1149

[Levi Weintraub]

It seems to me that Editor::appliedEditing or Editor::changeSelectionAfterCommand shouldn't change the focused node, so it should perhaps ensure it's trying to update the selection on the currently focused node and bail if it's not? I'll see if that behavior matches FFX before assuming that doesn't break anything else.

[Levi Weintraub]

(In reply to comment #5)
> It seems to me that Editor::appliedEditing or Editor::changeSelectionAfterCommand shouldn't change the focused node, so it should perhaps ensure it's trying to update the selection on the currently focused node and bail if it's not? I'll see if that behavior matches FFX before assuming that doesn't break anything else.

I tried a basic fix in Editor::changeSelectionAfterCommand, where I only change selection if the new selection occurs in the same context as the currently focused node. This does cause some changes to selection behavior in our already present layout tests. Aside from EditingDelegate changes, selections when dragging content changes: the content dragged in isn't selected.

This is like FFX (though notably FFX doesn't support dragging from one editable context to another... the original contents are lost and not pasted in!). 

IE9 highlights the dragged text in the destination, but doesn't support the onInput event listener. Trying to use "addEventListener" as they describe in their documentation (http://msdn.microsoft.com/en-us/library/gg592978(v=vs.85).aspx) results in a javascript exception stating that the input tag doesn't support "addEventListener".

I'm inclined towards changing our behavior, but understand that these changes may make it less than desirable. Opinions?

[Levi Weintraub]

(In reply to comment #6)
> IE9 highlights the dragged text in the destination, but doesn't support the onInput event listener. Trying to use "addEventListener" as they describe in their documentation (http://msdn.microsoft.com/en-us/library/gg592978(v=vs.85).aspx) results in a javascript exception stating that the input tag doesn't support "addEventListener".

Ryosuke pointed out that I ran into this because I was in Quirks mode (oops!). IE9 then passes the attached test case.

[Levi Weintraub]

After some more testing, I've identified that IE9 and Firefox do not set the focus when setting the selection, which SelectionController::setFocus does indiscriminately. Fixing this behavior should also fix this bug. The fix should be easy, but call sites that expect the focus to be set along with selection will need to be updated to respect the new contract, and lots of layout tests will have to be rebaselined with the changed editing delegates.

I'm happy to rename the bug and take it, but I'll defer to Alice if she still wants to fix it.

[Alice Boxhall]

(In reply to comment #8)
> After some more testing, I've identified that IE9 and Firefox do not set the focus when setting the selection, which SelectionController::setFocus does indiscriminately. Fixing this behavior should also fix this bug. The fix should be easy, but call sites that expect the focus to be set along with selection will need to be updated to respect the new contract, and lots of layout tests will have to be rebaselined with the changed editing delegates.
> 
> I'm happy to rename the bug and take it, but I'll defer to Alice if she still wants to fix it.

I had a look at this bug with Ojan yesterday and we found the same thing.

I'm unwell today so I wouldn't get to it till tomorrow at the earliest, so I'm just as happy for you to take it if you'd like.

[Levi Weintraub]

Created attachment 86008 [details]
Work in progress

This affects the editing delegates for tons of tests, so it'll be a large rebaselining effort. I'll also add a test case.

This has the possibility of breaking compatibility with sites that rely on WebKit's current behavior. I'd love to get confirmation that we think this change is the right way to go. Darin? AP?

[Ahmad Saleem]

I am unable to reproduce this in Safari 15.5 on macOS 12.4 using attached "demo". As soon as I type anything after first letter, the focus shift to next one and the text keep on appearing in second input element. It is consistent with other browsers Firefox Nightly 103 and Chrome Canary 104. Thanks!

[Alexey Proskuryakov]

Thank you for checking! CC'ed a few folks to also verify the behavior in case they have concerns.

[Radar WebKit Bug Importer]

<rdar://problem/94223857>