Bug 977029 - when color value changes, refresh color control frame instead of (wrongly) changing value attribute. r=bz
authorArnaud Bienner <arnaud.bienner@gmail.com>
Fri, 28 Feb 2014 20:52:42 +0100
changeset 171728 6ed443a0daa3f748bb3e288618c5a68c2f78e8a4
parent 171727 20c705d00e7c48ff5c82558af56be53a1a8f7f4c
child 171729 90fb37782bb52c9fc3f7bcbf2e79739287b77cbe
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersbz
bugs977029
milestone30.0a1
Bug 977029 - when color value changes, refresh color control frame instead of (wrongly) changing value attribute. r=bz
content/html/content/src/HTMLInputElement.cpp
content/html/content/test/forms/mochitest.ini
content/html/content/test/forms/test_input_defaultValue.html
layout/forms/nsColorControlFrame.cpp
layout/forms/nsColorControlFrame.h
layout/reftests/forms/input/color/block-invalidate-2-ref.html
layout/reftests/forms/input/color/block-invalidate-2.html
layout/reftests/forms/input/color/reftest.list
--- a/content/html/content/src/HTMLInputElement.cpp
+++ b/content/html/content/src/HTMLInputElement.cpp
@@ -15,16 +15,17 @@
 #include "nsITextControlElement.h"
 #include "nsIDOMNSEditableElement.h"
 #include "nsIRadioVisitor.h"
 #include "nsIPhonetic.h"
 
 #include "nsIControllers.h"
 #include "nsIStringBundle.h"
 #include "nsFocusManager.h"
+#include "nsColorControlFrame.h"
 #include "nsNumberControlFrame.h"
 #include "nsPIDOMWindow.h"
 #include "nsRepeatService.h"
 #include "nsContentCID.h"
 #include "nsIComponentManager.h"
 #include "nsIDOMHTMLFormElement.h"
 #include "nsIDOMProgressEvent.h"
 #include "nsGkAtoms.h"
@@ -2800,22 +2801,22 @@ HTMLInputElement::SetValueInternal(const
             do_QueryFrame(GetPrimaryFrame());
           if (numberControlFrame) {
             numberControlFrame->SetValueOfAnonTextControl(value);
           }
         }
         OnValueChanged(!mParserCreating);
       }
 
-      // Call parent's SetAttr for color input so its control frame is notified
-      // and updated
       if (mType == NS_FORM_INPUT_COLOR) {
-        return nsGenericHTMLFormElement::SetAttr(kNameSpaceID_None,
-                                                 nsGkAtoms::value, aValue,
-                                                 true);
+        // Update color frame, to reflect color changes
+        nsColorControlFrame* colorControlFrame = do_QueryFrame(GetPrimaryFrame());
+        if (colorControlFrame) {
+          colorControlFrame->UpdateColor();
+        }
       }
 
       return NS_OK;
     }
 
     case VALUE_MODE_DEFAULT:
     case VALUE_MODE_DEFAULT_ON:
       // If the value of a hidden input was changed, we mark it changed so that we
--- a/content/html/content/test/forms/mochitest.ini
+++ b/content/html/content/test/forms/mochitest.ini
@@ -15,16 +15,17 @@ support-files =
 [test_form_attributes_reflection.html]
 [test_formaction_attribute.html]
 [test_formnovalidate_attribute.html]
 [test_input_attributes_reflection.html]
 [test_input_color_input_change_events.html]
 [test_input_color_picker_initial.html]
 [test_input_color_picker_popup.html]
 [test_input_color_picker_update.html]
+[test_input_defaultValue.html]
 [test_input_email.html]
 [test_input_event.html]
 [test_input_file_picker.html]
 [test_input_list_attribute.html]
 [test_input_number_l10n.html]
 # We don't build ICU for Firefox for Android or Firefox OS:
 skip-if = os == "android" || appname == "b2g"
 [test_input_number_key_events.html]
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/forms/test_input_defaultValue.html
@@ -0,0 +1,81 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=977029
+-->
+<head>
+  <title>Test for Bug 977029</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+</head>
+<body>
+<div id="content">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=977029">Bug 977029</a>
+  <p>
+    Goal of this test is to check that modifying defaultValue and value attribute
+    of input types is working as expected.
+  </p>
+  <form>
+    <input id='a' type="color" value="#00ff00">
+    <input id='b' type="text" value="foo">
+    <input id='c' type="email" value="foo">
+    <input id='d' type="date" value="2010-09-20">
+    <input id='e' type="search" value="foo">
+    <input id='f' type="tel" value="foo">
+    <input id='g' type="url" value="foo">
+    <input id='h' type="number" value="42">
+    <input id='i' type="range" value="42" min="0" max="100">
+    <input id='j' type="time" value="17:00:25.54">
+  </form>
+</div>
+<script type="application/javascript">
+
+// [ element id | original defaultValue | another value | another default value]
+// Preferably use only valid values: the goal of this test isn't to test the 
+// value sanitization algorithm (for input types which have one) as this is
+// already part of another test)
+var testData = [["a", "#00ff00", "#00aaaa", "#00ccaa"],
+                ["b", "foo", "bar", "tulip"],
+                ["c", "foo", "foo@bar.org", "tulip"],
+                ["d", "2010-09-20", "2012-09-21", ""],
+                ["e", "foo", "bar", "tulip"],
+                ["f", "foo", "bar", "tulip"],
+                ["g", "foo", "bar", "tulip"],
+                ["h", "42", "1337", "3"],
+                ["i", "42", "17", "3"],
+                ["j", "17:00:25.54", "07:00:25", "03:00:03"],
+               ];
+
+for (var data of testData) {
+  id = data[0];
+  input = document.getElementById(id);
+  originalDefaultValue = data[1];
+  is(originalDefaultValue, input.defaultValue,
+    "Default value isn't the expected one");
+  is(originalDefaultValue, input.value,
+    "input.value original value is different from defaultValue");
+  input.defaultValue = data[2]
+  is(input.defaultValue, input.value,
+    "Changing default value before value was changed should change value too");
+  input.value = data[3];
+  input.defaultValue = originalDefaultValue;
+  is(input.value, data[3],
+    "Changing default value after value was changed should not change value");
+  input.value = data[2];
+  is(originalDefaultValue, input.defaultValue,
+    "defaultValue shouldn't change when changing value");
+  input.defaultValue = data[3];
+  is(input.defaultValue, data[3],
+    "defaultValue should have changed");
+  // Change the value...
+  input.value = data[2];
+  is(input.value, data[2],
+    "value should have changed");
+  // ...then reset the form
+  input.form.reset();
+  is(input.defaultValue, input.value,
+    "reset form should bring back the default value");
+}
+</script>
+</body>
+</html>
+
--- a/layout/forms/nsColorControlFrame.cpp
+++ b/layout/forms/nsColorControlFrame.cpp
@@ -26,16 +26,17 @@ nsIFrame*
 NS_NewColorControlFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
   return new (aPresShell) nsColorControlFrame(aContext);
 }
 
 NS_IMPL_FRAMEARENA_HELPERS(nsColorControlFrame)
 
 NS_QUERYFRAME_HEAD(nsColorControlFrame)
+  NS_QUERYFRAME_ENTRY(nsColorControlFrame)
   NS_QUERYFRAME_ENTRY(nsIAnonymousContentCreator)
 NS_QUERYFRAME_TAIL_INHERITING(nsColorControlFrameSuper)
 
 
 void nsColorControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
   nsContentUtils::DestroyAnonymousContent(&mColorContent);
--- a/layout/forms/nsColorControlFrame.h
+++ b/layout/forms/nsColorControlFrame.h
@@ -20,18 +20,19 @@ class nsColorControlFrame MOZ_FINAL : pu
   typedef mozilla::dom::Element Element;
 
 public:
   friend nsIFrame* NS_NewColorControlFrame(nsIPresShell* aPresShell,
                                            nsStyleContext* aContext);
 
   virtual void DestroyFrom(nsIFrame* aDestructRoot) MOZ_OVERRIDE;
 
+  NS_DECL_QUERYFRAME_TARGET(nsColorControlFrame)
+  NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS
-  NS_DECL_QUERYFRAME
 
   virtual nsIAtom* GetType() const MOZ_OVERRIDE;
 
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const MOZ_OVERRIDE;
 #endif
 
   // nsIAnonymousContentCreator
@@ -43,19 +44,19 @@ public:
   virtual nsresult AttributeChanged(int32_t  aNameSpaceID,
                                     nsIAtom* aAttribute,
                                     int32_t  aModType) MOZ_OVERRIDE;
   virtual bool IsLeaf() const MOZ_OVERRIDE { return true; }
   virtual nsIFrame* GetContentInsertionFrame() MOZ_OVERRIDE;
 
   virtual Element* GetPseudoElement(nsCSSPseudoElements::Type aType) MOZ_OVERRIDE;
 
+  // Refresh the color swatch, using associated input's value
+  nsresult UpdateColor();
+
 private:
   nsColorControlFrame(nsStyleContext* aContext);
 
-  // Update the color swatch
-  nsresult UpdateColor();
-
   nsCOMPtr<Element> mColorContent;
 };
 
 
 #endif // nsColorControlFrame_h___
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/input/color/block-invalidate-2-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <p>Test for bug <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=977038">977038</a></p>
+    <form>
+      <input type="color" value="#00ff00" />
+    </form>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/input/color/block-invalidate-2.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html class='reftest-wait'>
+  <script>
+    function runTest() {
+      var p = document.getElementsByTagName('input')[0];
+      p.value = '#0000ff'
+      p.defaultValue = '#00ff00';
+      p.form.reset();
+      document.documentElement.className = '';
+    }
+    window.addEventListener("MozReftestInvalidate", runTest, false);
+  </script>
+  <body>
+    <p>Test for bug <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=977038">977038</a></p>
+    <form>
+      <input type="color" />
+    </form>
+  </body>
+</html>
--- a/layout/reftests/forms/input/color/reftest.list
+++ b/layout/reftests/forms/input/color/reftest.list
@@ -4,11 +4,12 @@
 fails-if(B2G||Android) == input-color-1.html input-color-1-ref.html
 
 default-preferences pref(dom.forms.color,true)
 
 # Despite the "default-preferences" line above, B2G and Android are still
 # excluded from some style in forms.css, which makes the following tests fail.
 fails-if(B2G||Android) == margin-padding-1.html margin-padding-1-ref.html
 == block-invalidate-1.html block-invalidate-1-ref.html
+== block-invalidate-2.html block-invalidate-2-ref.html
 fails-if(B2G||Android) == transformations-1.html transformations-1-ref.html
 fails-if(B2G||Android) == custom-style-1.html custom-style-1-ref.html
 fails-if(B2G||Android) == custom-style-2.html custom-style-2-ref.html