Bug 997805 - Correctly restore the placeholder text after the editor object is re-attached to a text control as a result of a reframe; r=bzbarsky
☠☠ backed out by 7a633ff64906 ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 25 Apr 2014 16:40:44 -0400
changeset 180682 669fdba2ceef8651844b672b1b3117e5386e54ce
parent 180681 ea4e380efa0ad3422e017a0e4c74cc61c9ed5187
child 180683 6b04f015e541e07ff631ac18473824f7fb065755
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersbzbarsky
bugs997805
milestone31.0a1
Bug 997805 - Correctly restore the placeholder text after the editor object is re-attached to a text control as a result of a reframe; r=bzbarsky Recomputing the placeholder visibility does not require the placeholder div itself to be present, as the only information required for that is the current value of the text control which is present either way. This patch fixes nsTextEditorState::ValueWasChanged and nsTextEditorState::UpdatePlaceholderVisibility to that effect. But the real fix is in nsTextEditorState::UpdatePlaceholderText, where after setting the placeholder text on the anonymous div, we redo the placeholder visibility computation. Since this function can be called from HTMLTextAreaElement::CreatePlaceholderNode during frame construction, the GetValue function may return the wrong value since the editor has not properly been set up yet, resulting in this bug. And this function call is useless anyway, because changing the placeholder text does not really affect the result of the visibility computation, so there is no need to do this work in the first place.
content/html/content/src/nsTextEditorState.cpp
editor/reftests/997805-ref.html
editor/reftests/997805.html
editor/reftests/reftest.list
--- a/content/html/content/src/nsTextEditorState.cpp
+++ b/content/html/content/src/nsTextEditorState.cpp
@@ -1934,21 +1934,16 @@ nsTextEditorState::InitializeKeyboardEve
   }
 
   mSelCon->SetScrollableFrame(do_QueryFrame(mBoundFrame->GetFirstPrincipalChild()));
 }
 
 void
 nsTextEditorState::ValueWasChanged(bool aNotify)
 {
-  // placeholder management
-  if (!mPlaceholderDiv) {
-    return;
-  }
-
   UpdatePlaceholderVisibility(aNotify);
 }
 
 void
 nsTextEditorState::UpdatePlaceholderText(bool aNotify)
 {
   NS_ASSERTION(mPlaceholderDiv, "This function should not be called if "
                                 "mPlaceholderDiv isn't set");
@@ -1959,25 +1954,21 @@ nsTextEditorState::UpdatePlaceholderText
 
   nsAutoString placeholderValue;
 
   nsCOMPtr<nsIContent> content = do_QueryInterface(mTextCtrlElement);
   content->GetAttr(kNameSpaceID_None, nsGkAtoms::placeholder, placeholderValue);
   nsContentUtils::RemoveNewlines(placeholderValue);
   NS_ASSERTION(mPlaceholderDiv->GetFirstChild(), "placeholder div has no child");
   mPlaceholderDiv->GetFirstChild()->SetText(placeholderValue, aNotify);
-  ValueWasChanged(aNotify);
 }
 
 void
 nsTextEditorState::UpdatePlaceholderVisibility(bool aNotify)
 {
-  NS_ASSERTION(mPlaceholderDiv, "This function should not be called if "
-                                "mPlaceholderDiv isn't set");
-
   nsAutoString value;
   GetValue(value, true);
 
   mPlaceholderVisibility = value.IsEmpty();
 
   if (mPlaceholderVisibility &&
       !Preferences::GetBool("dom.placeholder.show_on_focus", true)) {
     nsCOMPtr<nsIContent> content = do_QueryInterface(mTextCtrlElement);
new file mode 100644
--- /dev/null
+++ b/editor/reftests/997805-ref.html
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<textarea placeholder="placeholder"></textarea>
new file mode 100644
--- /dev/null
+++ b/editor/reftests/997805.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<textarea placeholder="placeholder"></textarea>
+<script>
+onload = function() {
+  var t = document.querySelector("textarea");
+  t.style.display = "none";
+  t.value = "test";
+  setTimeout(function() {
+    t.style.display = "";
+    t.value = "";
+    document.documentElement.className = "";
+  }, 0);
+};
+</script>
+</html>
--- a/editor/reftests/reftest.list
+++ b/editor/reftests/reftest.list
@@ -121,8 +121,9 @@ needs-focus == spellcheck-contenteditabl
 == spellcheck-contenteditable-attr-dynamic-inherit.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-property-dynamic.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-property-dynamic-inherit.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-attr-dynamic-override.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-attr-dynamic-override-inherit.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-property-dynamic-override.html spellcheck-contenteditable-disabled-ref.html
 == spellcheck-contenteditable-property-dynamic-override-inherit.html spellcheck-contenteditable-disabled-ref.html
 needs-focus == 969773.html 969773-ref.html
+== 997805.html 997805-ref.html