Bug 615833 - Change event should not be cancelable. r=smaug a=sicking
authorMounir Lamouri <mounir.lamouri@gmail.com>
Fri, 17 Dec 2010 09:45:46 -0800
changeset 59448 5370d662ae51a18c353546e7e8b19e3ad663f4db
parent 59447 98ee88c41e2377c4464102cf6aaeffe6d216e9e8
child 59449 0ae68f7446c014ecd951156c70f1ddb197e66ee4
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewerssmaug, sicking
bugs615833
milestone2.0b9pre
Bug 615833 - Change event should not be cancelable. r=smaug a=sicking
content/html/content/src/nsHTMLInputElement.cpp
content/html/content/src/nsHTMLInputElement.h
content/html/content/test/Makefile.in
content/html/content/test/test_bug592802.html
content/html/content/test/test_bug615833.html
layout/forms/nsListControlFrame.cpp
layout/forms/nsTextControlFrame.cpp
layout/reftests/bugs/557087-1.html
layout/reftests/bugs/557087-2.html
--- a/content/html/content/src/nsHTMLInputElement.cpp
+++ b/content/html/content/src/nsHTMLInputElement.cpp
@@ -439,17 +439,17 @@ AsyncClickHandler::Run()
   if (newFiles.Count()) {
     // The text control frame (if there is one) isn't going to send a change
     // event because it will think this is done by a script.
     // So, we can safely send one by ourself.
     mInput->SetFiles(newFiles, true);
     nsContentUtils::DispatchTrustedEvent(mInput->GetOwnerDoc(),
                                          static_cast<nsIDOMHTMLInputElement*>(mInput.get()),
                                          NS_LITERAL_STRING("change"), PR_TRUE,
-                                         PR_TRUE);
+                                         PR_FALSE);
   }
 
   return NS_OK;
 }
 
 #define CPS_PREF_NAME NS_LITERAL_STRING("browser.upload.lastDir")
 
 NS_IMPL_ISUPPORTS2(UploadLastDir, nsIObserver, nsISupportsWeakReference)
@@ -1714,30 +1714,16 @@ nsHTMLInputElement::SetCheckedInternal(P
   if (mType == NS_FORM_INPUT_RADIO) {
     // OnValueChanged is going to be called for all radios in the radio group.
     nsCOMPtr<nsIRadioVisitor> visitor =
       NS_GetRadioUpdateValueMissingVisitor();
     VisitGroup(visitor, aNotify);
   }
 }
 
-
-void
-nsHTMLInputElement::FireOnChange()
-{
-  //
-  // Since the value is changing, send out an onchange event (bug 23571)
-  //
-  nsEventStatus status = nsEventStatus_eIgnore;
-  nsEvent event(PR_TRUE, NS_FORM_CHANGE);
-  nsRefPtr<nsPresContext> presContext = GetPresContext();
-  nsEventDispatcher::Dispatch(static_cast<nsIContent*>(this), presContext,
-                              &event, nsnull, &status);
-}
-
 NS_IMETHODIMP
 nsHTMLInputElement::Blur()
 {
   return nsGenericHTMLElement::Blur();
 }
 
 NS_IMETHODIMP
 nsHTMLInputElement::Focus()
@@ -2229,17 +2215,20 @@ nsHTMLInputElement::PostHandleEvent(nsEv
         }
       } else if (oldType == NS_FORM_INPUT_CHECKBOX) {
         PRBool originalIndeterminateValue =
           !!(aVisitor.mItemFlags & NS_ORIGINAL_INDETERMINATE_VALUE);
         SetIndeterminateInternal(originalIndeterminateValue, PR_FALSE);
         DoSetChecked(originalCheckedValue, PR_TRUE, PR_TRUE);
       }
     } else {
-      FireOnChange();
+      nsContentUtils::DispatchTrustedEvent(GetOwnerDoc(),
+                                           static_cast<nsIDOMHTMLInputElement*>(this),
+                                           NS_LITERAL_STRING("change"), PR_TRUE,
+                                           PR_FALSE);
 #ifdef ACCESSIBILITY
       // Fire an event to notify accessibility
       if (mType == NS_FORM_INPUT_CHECKBOX) {
         FireEventForAccessibility(this, aVisitor.mPresContext,
                                   NS_LITERAL_STRING("CheckboxStateChange"));
       } else {
         FireEventForAccessibility(this, aVisitor.mPresContext,
                                   NS_LITERAL_STRING("RadioStateChange"));
--- a/content/html/content/src/nsHTMLInputElement.h
+++ b/content/html/content/src/nsHTMLInputElement.h
@@ -415,21 +415,16 @@ protected:
   }
 
   virtual PRBool AcceptAutofocus() const
   {
     return PR_TRUE;
   }
 
   /**
-   * Fire the onChange event
-   */
-  void FireOnChange();
-
-  /**
    * Visit the group of radio buttons this radio belongs to
    * @param aVisitor the visitor to visit with
    */
   nsresult VisitGroup(nsIRadioVisitor* aVisitor, PRBool aFlushContent);
 
   /**
    * Do all the work that |SetChecked| does (radio button handling, etc.), but
    * take an |aNotify| parameter.
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -246,12 +246,13 @@ include $(topsrcdir)/config/rules.mk
 		test_bug613113.html \
 		test_bug605124-1.html \
 		test_bug605124-2.html \
 		test_bug605125-1.html \
 		test_bug605125-2.html \
 		test_bug612730.html \
 		test_bug613722.html \
 		test_bug613979.html \
+		test_bug615833.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
--- a/content/html/content/test/test_bug592802.html
+++ b/content/html/content/test/test_bug592802.html
@@ -181,17 +181,17 @@ SimpleTest.waitForFocus(function() {
 
   // Now, we can launch tests when file picker isn't canceled.
   var b = document.getElementById('b');
   b.focus(); // Be sure the element is visible.
 
   document.getElementById('a').addEventListener("change", function(aEvent) {
     ok(true, "change event correctly sent");
     ok(aEvent.bubbles, "change event should bubble");
-    todo(!aEvent.cancelable, "change event should not be cancelable");
+    ok(!aEvent.cancelable, "change event should not be cancelable");
     testCounter++;
 
     if (testCounter >= testNb) {
       aEvent.target.removeEventListener("change", arguments.callee, false);
       SimpleTest.executeSoon(finished);
     } else {
       var data = testData[testCounter];
       var a = document.getElementById('a');
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug615833.html
@@ -0,0 +1,145 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=615697
+-->
+<head>
+  <title>Test for Bug 615697</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=615697">Mozilla Bug 615697</a>
+<p id="display"></p>
+<div id="content">
+  <input>
+  <textarea></textarea>
+  <input type='radio'>
+  <input type='checkbox'>
+  <select>
+    <option>foo</option>
+    <option>bar</option>
+  </select>
+  <select multiple size='1'>
+    <option>tulip</option>
+  </select>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 615697 **/
+
+/**
+ * This test is making all elements trigger 'change' event.
+ * You should read the test from bottom to top:
+ * events are registered from the last one to the first one.
+ *
+ * Sometimes, elements are focused before a click. This might sound useless
+ * but it guarantees to have the element visible before simulating the click.
+ */
+
+var input = document.getElementsByTagName('input')[0];
+var textarea = document.getElementsByTagName('textarea')[0];
+var radio = document.getElementsByTagName('input')[1];
+var checkbox= document.getElementsByTagName('input')[2];
+var select = document.getElementsByTagName('select')[0];
+var selectMultiple = document.getElementsByTagName('select')[1];
+
+function checkChangeEvent(aEvent)
+{
+  ok(aEvent.bubbles, "change event should bubble");
+  ok(!aEvent.cancelable, "change event shouldn't be cancelable");
+}
+
+selectMultiple.addEventListener("change", function(aEvent) {
+  selectMultiple.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  SimpleTest.finish();
+}, false);
+
+selectMultiple.addEventListener("focus", function() {
+  selectMultiple.removeEventListener("focus", arguments.callee, false);
+  synthesizeMouseAtCenter(selectMultiple, {});
+}, false);
+
+select.addEventListener("change", function(aEvent) {
+  select.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  selectMultiple.focus();
+}, false);
+
+select.addEventListener("keyup", function() {
+  select.removeEventListener("keyup", arguments.callee, false);
+  select.blur();
+}, false);
+
+select.addEventListener("focus", function() {
+  select.removeEventListener("focus", arguments.callee, false);
+  synthesizeKey("VK_DOWN", {});
+}, false);
+
+checkbox.addEventListener("change", function(aEvent) {
+  checkbox.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  select.focus();
+}, false);
+
+checkbox.addEventListener("focus", function() {
+  checkbox.removeEventListener("focus", arguments.callee, false);
+  synthesizeMouseAtCenter(checkbox, {});
+}, false);
+
+radio.addEventListener("change", function(aEvent) {
+  radio.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  checkbox.focus();
+}, false);
+
+radio.addEventListener("focus", function() {
+  radio.removeEventListener("focus", arguments.callee, false);
+  synthesizeMouseAtCenter(radio, {});
+}, false);
+
+textarea.addEventListener("change", function(aEvent) {
+  textarea.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  radio.focus();
+}, false);
+
+textarea.addEventListener("input", function() {
+  textarea.removeEventListener("input", arguments.callee, false);
+  textarea.blur();
+}, false);
+
+textarea.addEventListener("focus", function() {
+  textarea.removeEventListener("focus", arguments.callee, false);
+  synthesizeKey('f', {});
+}, false);
+
+input.addEventListener("change", function(aEvent) {
+  input.removeEventListener("change", arguments.callee, false);
+  checkChangeEvent(aEvent);
+  textarea.focus();
+}, false);
+
+input.addEventListener("input", function() {
+  input.removeEventListener("input", arguments.callee, false);
+  input.blur();
+}, false);
+
+input.addEventListener("focus", function() {
+  input.removeEventListener("focus", arguments.callee, false);
+  synthesizeKey('f', {});
+}, false);
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  input.focus();
+});
+
+</script>
+</pre>
+</body>
+</html>
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -1623,24 +1623,20 @@ nsListControlFrame::FireOnChange()
     if (index == NS_SKIP_NOTIFY_INDEX)
       return;
 
     // See if the selection actually changed
     if (index == GetSelectedIndex())
       return;
   }
 
-  // Dispatch the NS_FORM_CHANGE event
-  nsEventStatus status = nsEventStatus_eIgnore;
-  nsEvent event(PR_TRUE, NS_FORM_CHANGE);
-
-  nsCOMPtr<nsIPresShell> presShell = PresContext()->GetPresShell();
-  if (presShell) {
-    presShell->HandleEventWithTarget(&event, this, nsnull, &status);
-  }
+  // Dispatch the change event.
+  nsContentUtils::DispatchTrustedEvent(mContent->GetOwnerDoc(), mContent,
+                                       NS_LITERAL_STRING("change"), PR_TRUE,
+                                       PR_FALSE);
 }
 
 NS_IMETHODIMP
 nsListControlFrame::OnSetSelectedIndex(PRInt32 aOldIndex, PRInt32 aNewIndex)
 {
   if (mComboboxFrame) {
     // UpdateRecentIndex with NS_SKIP_NOTIFY_INDEX, so that we won't fire an onchange
     // event for this setting of selectedIndex.
--- a/layout/forms/nsTextControlFrame.cpp
+++ b/layout/forms/nsTextControlFrame.cpp
@@ -1354,21 +1354,20 @@ nsTextControlFrame::InitFocusedValue()
 NS_IMETHODIMP
 nsTextControlFrame::CheckFireOnChange()
 {
   nsString value;
   GetText(value);
   if (!mFocusedValue.Equals(value))
   {
     mFocusedValue = value;
-    // Dispatch the change event
-    nsEventStatus status = nsEventStatus_eIgnore;
-    nsInputEvent event(PR_TRUE, NS_FORM_CHANGE, nsnull);
-    nsCOMPtr<nsIPresShell> shell = PresContext()->PresShell();
-    shell->HandleEventWithTarget(&event, nsnull, mContent, &status);
+    // Dispatch the change event.
+    nsContentUtils::DispatchTrustedEvent(mContent->GetOwnerDoc(), mContent,
+                                         NS_LITERAL_STRING("change"), PR_TRUE,
+                                         PR_FALSE);
   }
   return NS_OK;
 }
 
 // END IMPLEMENTING NS_IFORMCONTROLFRAME
 
 NS_IMETHODIMP
 nsTextControlFrame::SetInitialChildList(nsIAtom*        aListName,