Closed Bug 36573 Opened 20 years ago Closed 18 years ago

[DOM] onchange -> event.target is wrong element

Categories

(Core :: DOM: Events, defect)

x86
Other
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla0.9.9

People

(Reporter: joe, Assigned: joki)

References

Details

(Keywords: dom2, Whiteboard: [eapp?])

Attachments

(2 files, 1 obsolete file)

When an input element's onchange event fires, the target of the event should be 
a reference to the input element itself.  However, instead the target is the 
input's FORM element, or if the input element has no siblings, the HTML element.
Attached file simple test case
confirming
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is valid though the bug actually seems to be that the target of the current 
mouse event is being taken for the onchange event target.  I can see how that 
might happend.  I'll keep looking at it.
Adding [DOM] prefix to bugs related to DOM Level 0, 1, or 2 
compatibility/compliance.
Status: NEW → ASSIGNED
Summary: onchange -> event.target is wrong element → [DOM] onchange -> event.target is wrong element
Per heikki, saari, ckritzer:
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration.
Target Milestone: --- → Future
Blocks: 50660
Dragging and dropping a file onto a text input produces and event with an 
undefined target. The following INPUT field types do not work:
Text
File (Dropping on the 'Browse' button *does* work)
TEXTAREA

The following do work:
Submit
Checkbox
SELECT
Keywords: dom2
Component: DOM Level 2 → DOM Events
*** Bug 62798 has been marked as a duplicate of this bug. ***
Please change the target to 0.9 I tried doing it myself as per:
http://www.mozilla.org/roadmap.html "how you can help"/"Mozilla embedders"
but, apparently I'm not "sufficiently empowered" :-(
After much digging I found the problem and a solution.
The problem is as follows:
nsEventStateManager has a target mCurrentTarget which is set
by the mouse press event the problem is that for change event
this target is incorrect e.g.: the user enters text in the input
and than presses outside of the text input - the resulting target
is whatever the user pressed on.
The solution:
Set mCurrentTarget before dispatching the event, and reset it afterwards.
The code:
In nsEventStateManager::PreHandleEvent at the top add:
///////////////////////////
  if (!aPresContext && !aEvent && !aStatus && !aView && aTargetFrame)
    {
      mCurrentTarget = aTargetFrame;
      return NS_OK;
    }
////////////////////////////
In our case the only preprocessing needed is to set mCurrentTarget

In nsGfxTextControlFrame2::CallOnChange
add #include "nsIEventStateManager.h"
replace ... mContent->HandleDOMEvent ...
with:
/////////////////////////////
      {
        nsIFrame* oldTarget;
        nsCOMPtr<nsIEventStateManager> manager;
        context->GetEventStateManager(getter_AddRefs(manager));
        if (manager)
          {
            manager->GetEventTarget(&oldTarget);
            manager->PreHandleEvent(nsnull, nsnull, this, nsnull, nsnull);
          }
        nsresult result = mContent->HandleDOMEvent(context, &event, nsnull,
                                                   NS_EVENT_FLAG_INIT, &status);
        if (manager)
          manager->PreHandleEvent(nsnull, nsnull, oldTarget, nsnull, nsnull);
        return result;
      }
/////////////////////////////

Can someone please check this fix and submit it if OK ?
joki can you please take a look at the suggestion offered by Guy?
QA Contact Update
QA Contact: vidur → vladimire
jst, can you take a look at this patch please? thanks.
Assignee: joki → jst
Status: ASSIGNED → NEW
Keywords: mozilla0.9.4, patch
Priority: P3 → --
Target Milestone: Future → ---
This is really joki's code, I'd prefere if he'd have a look once he has a
chance. Back to joki.
Assignee: jst → joki
Attached patch G. Alfandary's fix as patch (obsolete) — Splinter Review
->0.9.6 for Tom to evaluate the patch
Target Milestone: --- → mozilla0.9.6
Too late for 0.9.6, this needs retargeting.
Keywords: mozilla0.9.7
0.9.6 has passed, moving to 0.9.7.  Load balancing 0.9.7 list shortly
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Moving bugs from 0.9.7 for triaging in 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Looking at the sequence of events as you tab out of the testcase's input field
to the select box gives:

================= begin event =================
Event called on obj [object HTMLInputElement]
[element[INPUT]]
evt[type]=change
evt[target]=[object HTMLSelectElement][element[SELECT]]
evt[currentTarget]=[object HTMLInputElement][element[INPUT]]
evt[eventPhase]=2
evt[bubbles]=true
evt[cancelable]=true
evt[view]=[object Window]
evt[originalTarget]=[object HTMLSelectElement][element[SELECT]]
evt[button]=65535
evt[rangeParent]=[object HTMLFormElement][element[FORM]]
evt[rangeOffset]=9
================= end event ===================

Which agrees with the earlier analysis. It seems to me that AT_TARGET, target ==
currentTarget should be an invariant.  What is the meaning of originalTarget?
Does this value of originalTarget==SELECT when tabbing from the INPUT to SELECT
make sense?

Getting event targets wrong is a bad thing.
Whiteboard: [eapp?]
Major corporations depend on eapp bugs, and need them to be fixed before they
can recommend Mozilla-based products to their customers. Adding nsbeta1+ keyword
and making sure the bugs get re-evaluated if they are targeted beyond 1.0.
Keywords: nsbeta1+
eapp was incorrectly used to change this to nsbeta1+. Resetting to nsbeta1 to
nominate. This is an important issue and deserves to be nsbeta1+ by the ADT.
Keywords: nsbeta1+nsbeta1
Note that after typing into the input field in the test case and either tab or
the mouse is used to transfer focus to the select box, the blur event fires on
the imput element but does not bubble before the focus event fires on the select
box.
The patch is definitely the right idea but there's actually a mechanism in
nsPresShell that achieves it more cleanly.  Posting new patch.
Attached patch Patch for bugSplinter Review
Attachment #48151 - Attachment is obsolete: true
Comment on attachment 70056 [details] [diff] [review]
Patch for bug

r=saari
Attachment #70056 - Flags: review+
Comment on attachment 70056 [details] [diff] [review]
Patch for bug

sr=jst
Attachment #70056 - Flags: superreview+
Fix checked in.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
verifying on build 2002-02-22-03-trunk windows 98
Status: RESOLVED → VERIFIED
If this is working can someone update
http://www.mozilla.org/docs/dom/mozilla/bug-events.html
You need to log in before you can comment on or make changes to this bug.