Closed Bug 615697 Opened 14 years ago Closed 14 years ago

file input's change event is not propagating during bubble

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: potch, Assigned: mounir)

References

()

Details

Attachments

(1 file, 1 obsolete file)

I came across this in one of the recent nightlies. In 3.6.x, I was able to set a change handler on a parent node of a file input, and the event bubbling worked properly. However, in the recent nightlies/beta 7, this is not working.

Selecting a file in the testcase in 3.6.x will trigger 4 handlers- 2 for the capture phase, two for the bubble phase. In 4.x, only 3 handlers are triggered- the delegate is not bubbled to.

I understand that the change event is a capture event by definition, but this is a regression from previous behavior, and I have not been able to find any documentation of when/why the change was made.
Regression range would be very useful here.
Ah, ok, I think I found it.
Assignee: nobody → Olli.Pettay
blocking2.0: --- → ?
Probably a regression from Bug 592802.
Component: DOM: Events → DOM: Core & HTML
QA Contact: events → general
Indeed, my bad.

This:
>    nsContentUtils::DispatchTrustedEvent(mInput->GetOwnerDoc(),
>                                         static_cast<nsIDOMHTMLInputElement*>(mInput.get()),
>                                         NS_LITERAL_STRING("change"), PR_FALSE,
>                                         PR_FALSE);

Should be changed to:
>    nsContentUtils::DispatchTrustedEvent(mInput->GetOwnerDoc(),
>                                         static_cast<nsIDOMHTMLInputElement*>(mInput.get()),
>                                         NS_LITERAL_STRING("change"), PR_TRUE,
>                                         PR_FALSE);

I very likely misread this: "the user agent must queue a task to first fire a simple event *that bubbles* named change at the input element" because simple events should not bubble by default.
Olli, feel free to assign me the bug.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
OK, I can review.
Assignee: Olli.Pettay → mounir.lamouri
Oh, and please add a testcase if possible.
And btw, I think both parameters should be PR_TRUE.
That is what change event seems to be in other cases in Gecko.
Better to test though.
(In reply to comment #7)
> Oh, and please add a testcase if possible.

Sure, as always.

(In reply to comment #8)
> And btw, I think both parameters should be PR_TRUE.

Indeed but is there a reason? This doesn't follow the specs nor other browser behavior.
> (In reply to comment #8)
> > And btw, I think both parameters should be PR_TRUE.
> 
> Indeed but is there a reason? This doesn't follow the specs nor other browser
> behavior.

Oups, I meant to quote "That is what change event seems to be in other cases in Gecko."
Attached patch Patch v1 (obsolete) — Splinter Review
I think the second test should be used for a follow up except if there is a reason to let cancel being cancelable.
Attachment #494191 - Flags: review?(Olli.Pettay)
Status: NEW → ASSIGNED
(In reply to comment #9)
> Indeed but is there a reason? This doesn't follow the specs nor other browser
> behavior.
Yeah, the only reason is consistency within Gecko.
Comment on attachment 494191 [details] [diff] [review]
Patch v1

So if I read the code right, "change" event is cancelable in other cases
(in Gecko), so for consistency please make it cancelable here too, and
then perhaps file a followup to change is non-cancelable everywhere.
That followup bug doesn't need to block 2.0.
Attachment #494191 - Flags: review?(Olli.Pettay) → review+
Depends on: 615833
Attached patch Patch v1.1Splinter Review
r=smuag

I will push the test for non file inputs with the follow-up.
Attachment #494191 - Attachment is obsolete: true
Whiteboard: [passed-try]
Blocking on this regression.
blocking2.0: ? → betaN+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/a1c92bbac6ac
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [passed-try]
Target Milestone: --- → mozilla2.0b8
No longer depends on: 615833
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: