Closed Bug 171210 Opened 22 years ago Closed 22 years ago

Dynamically adding form elements sends focus to lala land

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.4beta

People

(Reporter: msgregory, Assigned: bryner)

References

(Blocks 1 open bug)

Details

(Keywords: access, testcase, topembed+)

Attachments

(2 files)

When filling out a form, and some choice on the form changes the form's elements, the text caret either disappears completely (no amount of tabbing will bring it back to the form) or if the focus() method is used, the caret freezes and typing is ignored (including tab). Test case attached.
Attached file Test case
Hmmm, that's odd, it changed my code. Here it is again: (Oh by the way, I've tested this bug on Debian 1.0.0-3 and Windows 2000 1.0.1 and 1.2 alpha) <html> <head> <title>Form Test</title> </head> <body> Name: <input id=name> Role: <select id=role onchange='role_change()'> <option value=0>Role Model <option value=1>Super Model <option value=2>Programmer </select> <span id=attribute>Profession: <input id=profession> </span> <input type=button onclick='ok_click()' value=OK> <script language=javascript> function role_change() { var role = document.getElementById('role'); var attribute = document.getElementById('attribute'); while (attribute.hasChildNodes()) { attribute.removeChild(attribute.childNodes[0]); } var text, input; switch (role.value) { case '0': text = document.createTextNode('Profession: '); input = document.createElement('input'); input.id = 'profession'; break; case '1': text = document.createTextNode('Bust size: '); input = document.createElement('input'); input.id = 'bustsize'; break; case '2': text = document.createTextNode('Favorite Language: '); input = document.createElement('input'); input.id = 'favoritelanguage'; break; } attribute.appendChild(text); attribute.appendChild(input); // this line makes typing have no effect until the field is clicked on attribute.childNodes[1].focus(); // if it is left out, the cursor disappears altogether } </script> </body> </html>
worksforme, linux trunk build 20020926 and 1.2a
I suppose some instructions would help. 1. Load the page 2. type in a name 3. tab over to the role selector and press 'P' to change it 4. tab over to the next field to type in the programmer's favorite language. The caret will be stuck and you won't be able to type in the field without clicking on it. I discovered a workaround, though. At step 4, if you hit hit shift-tab to go back to the name field and then tab over twice to the newly created field, it will work. Or if the name and role fields are reversed it will work.
ah, now I see it. Problem exists with linux trunk build 20020926. I was using mouse before, which works fine. Since using the mouse works, ==> Keyboard
Assignee: jst → aaronl
Status: UNCONFIRMED → NEW
Component: DOM HTML → Keyboard Navigation
Ever confirmed: true
Keywords: testcase
QA Contact: stummala → sairuh
You can't even tab after that happens -- focus is gone.
Assignee: aaronl → bryner
Blocks: focusnav, atfmeta
OS: Linux → All
Hardware: PC → All
Summary: Dynamically adding form elements breaks text caret → Dynamically adding form elements sends focus to lala land
could this be similar / related to bug 170811?
Keywords: topembedtopembed+
Well, one thing that's happening is that the tab occurs before the onchange event fires, so it ends up tabbing to the text field that is then removed. I need to make sure this is the correct behavior. In addition, when a focused element is removed, we should be sending focus to the next element. I thought we were already doing this, but I guess not.
In this specific test case, the problem is that by the time the blur handler for the select is called, we've already decided which element we're going to focus next (this is, conincidentally, the element that will be yanked out of the document). So, there are a couple of possibilities: 1. blur the element before we locate the next thing to focus 2. after we blur the original element, double-check that what we think we're focusing next is still in the document, and if not, restart the search for something to focus from the original focused element. (1) could have unintended side-effects in cases where there is nothing else in the document to focus. it might be ok, but i'll need to investigate some more. I believe this is what IE does. (2) is safer, I think, but kind of hacky.
if you go for option 1, blurring the element before you locate the next focusable element you'll end up with cases where we blur and refocus the same element; cases where the next element isn't actually focusable. We should verify that an element is in the doc when we're doing the focusability check, and we should reset focus when we remove the focused element (which I also thought we were already doing...). Those 2 assertions should fix this, yes?
Right, the problem is that the element is in the document at the time we're checking whether it's focusable. It's during the blurring of the currently focused element that the element we're planning to focus is removed from the document. I don't think that trying to do something when the currently focused element is removed from the document is going to fix this. Two problems: 1. at that point, we've lost all track of whether we were shifting focus forward or backward 2. after the content is removed from the document, there's no way to find something near it to focus, since it's already removed from the content tree. You're that that option (1) that I mentioned could result in the same element being blurred and focused again, but that won't happen in real-world situations, because we'll at least go focus the document. The only situation I can think of where this could happen is in a XUL document.
Target Milestone: --- → mozilla1.4beta
Attached patch possible patchSplinter Review
Ensure that the element we've focused is still in the document after ChangeFocus() returns. If not, re-start the tabbing action from the old focused element. I don't _think_ this can infinitely recurse because we'll only run onblur event handlers for the old focused elememt once.
Attachment #112563 - Flags: review?(jkeiser)
Comment on attachment 112563 [details] [diff] [review] possible patch r=jkeiser. All I can say is thank god for nsCOMPtr.
Attachment #112563 - Flags: review?(jkeiser) → review+
Attachment #112563 - Flags: superreview?(jst)
Comment on attachment 112563 [details] [diff] [review] possible patch sr=jst
Attachment #112563 - Flags: superreview?(jst) → superreview+
Attachment #112563 - Flags: approval1.3b?
Comment on attachment 112563 [details] [diff] [review] possible patch a=asa (on behalf of drivers) for checkin to 1.3beta.
Attachment #112563 - Flags: approval1.3b? → approval1.3b+
checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
vrfy'd fixed with 2003.02.12 comm trunk bits on linux rh8.0, win2k and mac 10.2.3.
Status: RESOLVED → VERIFIED
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: