[Webkit-unassigned] [Bug 20780] when choosing an auto-completed value in a text input field the javascript 'change' event is not fired.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20780





--- Comment #14 from Darin Adler <darin at apple.com>  2009-11-01 14:01:42 PDT ---
Given that m_edited is used only for change events, I am sure that the change
in your patch is correct.

Pointing me at the related code I realize that there are things that would make
the code clearer:

    1) The three copies of the code to dispatch a change event if a text
control was edited should share code. RenderTextControl could have a public
function to do this and the isEdited and setEdited functions could be either
eliminated or made private.

    2) The rule for when a change event is dispatched seems wrong. I would
think there are other times we'd have to dispatch a change event. For example,
if the form is submitted some way other than by hitting return and there was an
edited <input> element in that form. Or if you are leaving the page for some
other reason, although the form element still has focus. We should write more
test cases and file bugs if we differ from other browsers.

    3) The Document::setFocusedNode and WMLInputElement::defaultEventHandler
functions should use the dispatchFormControlChangeEvent function.

    4) The code in Document::setFocusedNode to specifically deal with the text
controls should instead be in setFocus function overrides in the two classes
that can have text controls (HTMLInputElement, HTMLTextAreaElement), three
counting WMLInputElement. It is bad factoring to have element-specific code in
Document::setFocusedNode, even though it does save a few lines of code.

    5) The call to setEdited(false) should be done before dispatching the
event. It’s not clear to me that there’s any benefit to how it’s done now and I
think we might be able to construct a test case demonstrating some minor bug.
Even if we don't put code to dispatch the event into RenderTextControl as I
suggest in (3) above, perhaps we should at least make the function that returns
the bit also clear it as a side effect.

    6) For m_edited I would prefer a name that makes the relationship with the
change event clear. Perhaps m_wasChangedSinceLastChangedEvent. For the same
reason, m_userEdited could be renamed m_wasEditedSinceSetProgrammatically. Or
better, shorter names with a similar meaning. Both of my suggested names are
wordy, but m_edited and m_userEdited are too vague.

None of these changes need to go into your bug fix -- just things I noticed
because I had to read the code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.