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)
Core
DOM: UI Events & Focus Handling
Tracking
()
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: msgregory, Assigned: bryner)
References
(Blocks 1 open bug)
Details
(Keywords: access, testcase, topembed+)
Attachments
(2 files)
1.36 KB,
text/html
|
Details | |
1.11 KB,
patch
|
john
:
review+
jst
:
superreview+
asa
:
approval1.3b+
|
Details | Diff | Splinter Review |
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.
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>
Comment 3•22 years ago
|
||
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.
Comment 5•22 years ago
|
||
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
Comment 6•22 years ago
|
||
You can't even tab after that happens -- focus is gone.
Comment 7•22 years ago
|
||
could this be similar / related to bug 170811?
Updated•22 years ago
|
Assignee | ||
Comment 8•22 years ago
|
||
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.
Assignee | ||
Comment 9•22 years ago
|
||
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.
Comment 10•22 years ago
|
||
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?
Assignee | ||
Comment 11•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Target Milestone: --- → mozilla1.4beta
Assignee | ||
Comment 12•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Attachment #112563 -
Flags: review?(jkeiser)
Comment 13•22 years ago
|
||
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+
Assignee | ||
Updated•22 years ago
|
Attachment #112563 -
Flags: superreview?(jst)
Comment 14•22 years ago
|
||
Comment on attachment 112563 [details] [diff] [review]
possible patch
sr=jst
Attachment #112563 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Updated•22 years ago
|
Attachment #112563 -
Flags: approval1.3b?
Comment 15•22 years ago
|
||
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+
Assignee | ||
Comment 16•22 years ago
|
||
checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 17•22 years ago
|
||
vrfy'd fixed with 2003.02.12 comm trunk bits on linux rh8.0, win2k and mac 10.2.3.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Component: Keyboard: Navigation → User events and focus handling
Comment 18•6 years ago
|
||
Keywords: sec508
You need to log in
before you can comment on or make changes to this bug.
Description
•