Open Bug 304751 Opened 15 years ago Updated 12 years ago
On Change() inside ns Combobox Control Frame::Set Focus() is bad
From bug 258285 comment 213: As far as I can see this part of the patch is only necessary to fix the testcase "Tiny testcase.." (attachment 192321 [details]). The reason is that when we first blur the SELECT we end up here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/html/content/src/nsHTMLSelectElement.cpp&rev=1.232&root=/cvsroot&mark=1759-1772#1731 and on the "forced" frame blur on line 1767 we end up here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/forms/nsComboboxControlFrame.cpp&rev=1.324&root=/cvsroot&mark=509,519#508 and then the fun begins since we have onchange="something_else.focus()" which will move the focus and then "on the way back" we end up blurring what was just focused (in nsHTMLSelectElement::HandleDOMEvent line 1770). The placement of "FireOnChange()" inside "SetFocus()" is bad for other reasons as well and I was planning on moving it. Hopefully this solves the combobox blur problem too. The rest of the patch looks good, except maybe for the removal of the null-check of |mDocument| in one place - why do we not need it there?
Would fixing this remove the need for esm::EnsureFocusSynchronization() ? The blur handler itself could change focus, right?
Did smaug's recent changes affect this?
Flags: blocking1.9a1? → blocking1.9+
My fix prevented the crash if display type was changed during 'change' event. Bug 234455 may have changed things too. I'm not quite sure what this bug is about.
Mats/Smaug - is this bug still relevant?
Removing blocking status until we get feedback.
You need to log in before you can comment on or make changes to this bug.