Bug 977029 - when color value changes, refresh color control frame instead of (wrongly) changing value attribute. r=bz

2014-02-28 20:52:42 +01:00 · 2014-02-28 20:52:42 +01:00 · 77105e90e9
--- a/content/html/content/src/HTMLInputElement.cpp
+++ b/content/html/content/src/HTMLInputElement.cpp
@ -20,6 +20,7 @@
 #include "nsIControllers.h"
 #include "nsIStringBundle.h"
 #include "nsFocusManager.h"
+#include "nsColorControlFrame.h"
 #include "nsNumberControlFrame.h"
 #include "nsPIDOMWindow.h"
 #include "nsRepeatService.h"
@ -2805,12 +2806,12 @@ HTMLInputElement::SetValueInternal(const nsAString& aValue,
        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;
--- a/content/html/content/test/forms/mochitest.ini
+++ b/content/html/content/test/forms/mochitest.ini
@ -20,6 +20,7 @@ support-files =
 [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]
--- a/content/html/content/test/forms/test_input_defaultValue.html
+++ 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
@ -31,6 +31,7 @@ NS_NewColorControlFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 NS_IMPL_FRAMEARENA_HELPERS(nsColorControlFrame)

 NS_QUERYFRAME_HEAD(nsColorControlFrame)
+  NS_QUERYFRAME_ENTRY(nsColorControlFrame)
  NS_QUERYFRAME_ENTRY(nsIAnonymousContentCreator)
 NS_QUERYFRAME_TAIL_INHERITING(nsColorControlFrameSuper)

--- a/layout/forms/nsColorControlFrame.h
+++ b/layout/forms/nsColorControlFrame.h
@ -25,8 +25,9 @@ public:

  virtual void DestroyFrom(nsIFrame* aDestructRoot) MOZ_OVERRIDE;

-  NS_DECL_FRAMEARENA_HELPERS
+  NS_DECL_QUERYFRAME_TARGET(nsColorControlFrame)
  NS_DECL_QUERYFRAME
+  NS_DECL_FRAMEARENA_HELPERS

  virtual nsIAtom* GetType() const MOZ_OVERRIDE;

@ -48,12 +49,12 @@ public:

  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;
 };

--- a/layout/reftests/forms/input/color/block-invalidate-2-ref.html
+++ 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>
--- a/layout/reftests/forms/input/color/block-invalidate-2.html
+++ 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
@ -9,6 +9,7 @@ default-preferences pref(dom.forms.color,true)
 # 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