Bug 553097 - Do not create a frame and DOM node for placeholder if not needed. r=ehsan,roc a=blocking

content/html/content/public/nsITextControlElement.h

@@ -174,6 +174,11 @@ public:
    */
   NS_IMETHOD_(nsIContent*) GetRootEditorNode() = 0;
 
+  /**
+   * Create the placeholder anonymous node for the text control and returns it.
+   */
+  NS_IMETHOD_(nsIContent*) CreatePlaceholderNode() = 0;
+
   /**
    * Get the placeholder anonymous node for the text control.
    */

content/html/content/src/nsHTMLInputElement.cpp

@@ -1289,6 +1289,17 @@ nsHTMLInputElement::GetRootEditorNode()
   return nsnull;
 }
 
+NS_IMETHODIMP_(nsIContent*)
+nsHTMLInputElement::CreatePlaceholderNode()
+{
+  nsTextEditorState *state = GetEditorState();
+  if (state) {
+    NS_ENSURE_SUCCESS(state->CreatePlaceholderNode(), nsnull);
+    return state->GetPlaceholderNode();
+  }
+  return nsnull;
+}
+
 NS_IMETHODIMP_(nsIContent*)
 nsHTMLInputElement::GetPlaceholderNode()
 {
@@ -2747,6 +2758,8 @@ nsHTMLInputElement::GetAttributeChangeHint(const nsIAtom* aAttribute,
   } else if (aAttribute == nsGkAtoms::size &&
              IsSingleLineTextControl(PR_FALSE)) {
     NS_UpdateHint(retval, NS_STYLE_HINT_REFLOW);
+  } else if (PlaceholderApplies() && aAttribute == nsGkAtoms::placeholder) {
+    NS_UpdateHint(retval, NS_STYLE_HINT_FRAMECHANGE);
   }
   return retval;
 }

content/html/content/src/nsHTMLInputElement.h

@@ -206,6 +206,7 @@ public:
   NS_IMETHOD_(void) UnbindFromFrame(nsTextControlFrame* aFrame);
   NS_IMETHOD CreateEditor();
   NS_IMETHOD_(nsIContent*) GetRootEditorNode();
+  NS_IMETHOD_(nsIContent*) CreatePlaceholderNode();
   NS_IMETHOD_(nsIContent*) GetPlaceholderNode();
   NS_IMETHOD_(void) UpdatePlaceholderText(PRBool aNotify);
   NS_IMETHOD_(void) SetPlaceholderClass(PRBool aVisible, PRBool aNotify);

content/html/content/src/nsHTMLTextAreaElement.cpp

@@ -155,6 +155,7 @@ public:
   NS_IMETHOD_(void) UnbindFromFrame(nsTextControlFrame* aFrame);
   NS_IMETHOD CreateEditor();
   NS_IMETHOD_(nsIContent*) GetRootEditorNode();
+  NS_IMETHOD_(nsIContent*) CreatePlaceholderNode();
   NS_IMETHOD_(nsIContent*) GetPlaceholderNode();
   NS_IMETHOD_(void) UpdatePlaceholderText(PRBool aNotify);
   NS_IMETHOD_(void) SetPlaceholderClass(PRBool aVisible, PRBool aNotify);
@@ -501,6 +502,13 @@ nsHTMLTextAreaElement::GetRootEditorNode()
   return mState->GetRootNode();
 }
 
+NS_IMETHODIMP_(nsIContent*)
+nsHTMLTextAreaElement::CreatePlaceholderNode()
+{
+  NS_ENSURE_SUCCESS(mState->CreatePlaceholderNode(), nsnull);
+  return mState->GetPlaceholderNode();
+}
+
 NS_IMETHODIMP_(nsIContent*)
 nsHTMLTextAreaElement::GetPlaceholderNode()
 {
@@ -624,6 +632,8 @@ nsHTMLTextAreaElement::GetAttributeChangeHint(const nsIAtom* aAttribute,
     NS_UpdateHint(retval, NS_STYLE_HINT_REFLOW);
   } else if (aAttribute == nsGkAtoms::wrap) {
     NS_UpdateHint(retval, nsChangeHint_ReconstructFrame);
+  } else if (aAttribute == nsGkAtoms::placeholder) {
+    NS_UpdateHint(retval, NS_STYLE_HINT_FRAMECHANGE);
   }
   return retval;
 }

content/html/content/src/nsTextEditorState.cpp

@@ -1535,6 +1535,20 @@ nsTextEditorState::CreateRootNode()
 nsresult
 nsTextEditorState::CreatePlaceholderNode()
 {
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIContent> content = do_QueryInterface(mTextCtrlElement);
+    if (content) {
+      nsAutoString placeholderTxt;
+      content->GetAttr(kNameSpaceID_None, nsGkAtoms::placeholder,
+                       placeholderTxt);
+      nsContentUtils::RemoveNewlines(placeholderTxt);
+      NS_ASSERTION(!placeholderTxt.IsEmpty(), "CreatePlaceholderNode() shouldn't \
+be called if @placeholder is the empty string when trimmed from line breaks");
+    }
+  }
+#endif // DEBUG
+
   NS_ENSURE_TRUE(!mPlaceholderDiv, NS_ERROR_UNEXPECTED);
   NS_ENSURE_ARG_POINTER(mBoundFrame);
 
@@ -1861,6 +1875,10 @@ void
 nsTextEditorState::ValueWasChanged(PRBool aNotify)
 {
   // placeholder management
+  if (!mPlaceholderDiv) {
+    return;
+  }
+
   PRBool showPlaceholder = PR_FALSE;
   nsCOMPtr<nsIContent> content = do_QueryInterface(mTextCtrlElement);
   if (!nsContentUtils::IsFocusedContent(content)) {
@@ -1876,6 +1894,9 @@ nsTextEditorState::ValueWasChanged(PRBool aNotify)
 void
 nsTextEditorState::UpdatePlaceholderText(PRBool aNotify)
 {
+  NS_ASSERTION(mPlaceholderDiv, "This function should not be called if "
+                                "mPlaceholderDiv isn't set");
+
   // If we don't have a placeholder div, there's nothing to do.
   if (!mPlaceholderDiv)
     return;
@@ -1894,6 +1915,9 @@ void
 nsTextEditorState::SetPlaceholderClass(PRBool aVisible,
                                        PRBool aNotify)
 {
+  NS_ASSERTION(mPlaceholderDiv, "This function should not be called if "
+                                "mPlaceholderDiv isn't set");
+
   // No need to do anything if we don't have a frame yet
   if (!mBoundFrame)
     return;

content/html/content/src/nsTextEditorState.h

@@ -161,14 +161,14 @@ public:
   void EmptyValue() { if (mValue) mValue->Truncate(); }
   PRBool IsEmpty() const { return mValue ? mValue->IsEmpty() : PR_TRUE; }
 
+  nsresult CreatePlaceholderNode();
+
   nsIContent* GetRootNode() {
     if (!mRootNode)
       CreateRootNode();
     return mRootNode;
   }
   nsIContent* GetPlaceholderNode() {
-    if (!mPlaceholderDiv)
-      CreatePlaceholderNode();
     return mPlaceholderDiv;
   }
 
@@ -217,7 +217,6 @@ private:
   void operator= (const nsTextEditorState&);
 
   nsresult CreateRootNode();
-  nsresult CreatePlaceholderNode();
 
   void ValueWasChanged(PRBool aNotify);
 

layout/forms/nsTextControlFrame.cpp

@@ -422,6 +422,8 @@ nsTextControlFrame::EnsureEditorInitialized()
 nsresult
 nsTextControlFrame::CreateAnonymousContent(nsTArray<nsIContent*>& aElements)
 {
+  NS_ASSERTION(mContent, "We should have a content!");
+
   mState |= NS_FRAME_INDEPENDENT_SELECTION;
 
   nsCOMPtr<nsITextControlElement> txtCtrl = do_QueryInterface(GetContent());
@@ -437,11 +439,21 @@ nsTextControlFrame::CreateAnonymousContent(nsTArray<nsIContent*>& aElements)
   if (!aElements.AppendElement(rootNode))
     return NS_ERROR_OUT_OF_MEMORY;
 
-  nsIContent* placeholderNode = txtCtrl->GetPlaceholderNode();
-  NS_ENSURE_TRUE(placeholderNode, NS_ERROR_OUT_OF_MEMORY);
+  // Do we need a placeholder node?
+  nsAutoString placeholderTxt;
+  mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::placeholder,
+                    placeholderTxt);
+  nsContentUtils::RemoveNewlines(placeholderTxt);
+  mUsePlaceholder = !placeholderTxt.IsEmpty();
 
-  if (!aElements.AppendElement(placeholderNode))
-    return NS_ERROR_OUT_OF_MEMORY;
+  // Create the placeholder anonymous content if needed.
+  if (mUsePlaceholder) {
+    nsIContent* placeholderNode = txtCtrl->CreatePlaceholderNode();
+    NS_ENSURE_TRUE(placeholderNode, NS_ERROR_OUT_OF_MEMORY);
+
+    if (!aElements.AppendElement(placeholderNode))
+      return NS_ERROR_OUT_OF_MEMORY;
+  }
 
   rv = UpdateValueDisplay(PR_FALSE);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -659,17 +671,19 @@ void nsTextControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
   mScrollEvent.Revoke();
 
   if (!aOn) {
-    nsWeakFrame weakFrame(this);
+    if (mUsePlaceholder) {
+      PRInt32 textLength;
+      GetTextLength(&textLength);
 
-    PRInt32 length;
-    nsresult rv = GetTextLength(&length);
-    NS_ENSURE_SUCCESS(rv, );
-    if (!length)
-      txtCtrl->SetPlaceholderClass(PR_TRUE, PR_TRUE);
+      if (!textLength) {
+        nsWeakFrame weakFrame(this);
 
-    if (!weakFrame.IsAlive())
-    {
-      return;
+        txtCtrl->SetPlaceholderClass(PR_TRUE, PR_TRUE);
+
+        if (!weakFrame.IsAlive()) {
+          return;
+        }
+      }
     }
 
     MaybeEndSecureKeyboardInput();
@@ -680,13 +694,14 @@ void nsTextControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
   if (!selCon)
     return;
 
-  nsWeakFrame weakFrame(this);
+  if (mUsePlaceholder) {
+    nsWeakFrame weakFrame(this);
 
-  txtCtrl->SetPlaceholderClass(PR_FALSE, PR_TRUE);
+    txtCtrl->SetPlaceholderClass(PR_FALSE, PR_TRUE);
 
-  if (!weakFrame.IsAlive())
-  {
-    return;
+    if (!weakFrame.IsAlive()) {
+      return;
+    }
   }
 
   if (NS_SUCCEEDED(InitFocusedValue()))
@@ -1155,17 +1170,6 @@ nsTextControlFrame::AttributeChanged(PRInt32         aNameSpaceID,
                                      nsIAtom*        aAttribute,
                                      PRInt32         aModType)
 {
-  // First, check for the placeholder attribute, because it doesn't
-  // depend on the editor being present.
-  if (nsGkAtoms::placeholder == aAttribute)
-  {
-    nsWeakFrame weakFrame(this);
-    nsCOMPtr<nsITextControlElement> txtCtrl = do_QueryInterface(GetContent());
-    NS_ASSERTION(txtCtrl, "Content not a text control element");
-    txtCtrl->UpdatePlaceholderText(PR_TRUE);
-    NS_ENSURE_STATE(weakFrame.IsAlive());
-  }
-
   nsCOMPtr<nsITextControlElement> txtCtrl = do_QueryInterface(GetContent());
   NS_ASSERTION(txtCtrl, "Content not a text control element");
   nsISelectionController* selCon = txtCtrl->GetSelectionController();
@@ -1404,7 +1408,8 @@ nsTextControlFrame::UpdateValueDisplay(PRBool aNotify,
   NS_PRECONDITION(rootNode, "Must have a div content\n");
   NS_PRECONDITION(!mUseEditor,
                   "Do not call this after editor has been initialized");
-  NS_ASSERTION(txtCtrl->GetPlaceholderNode(), "A placeholder div must exist");
+  NS_ASSERTION(!mUsePlaceholder || txtCtrl->GetPlaceholderNode(),
+               "A placeholder div must exist");
 
   nsIContent *textContent = rootNode->GetChildAt(0);
   if (!textContent) {
@@ -1433,7 +1438,7 @@ nsTextControlFrame::UpdateValueDisplay(PRBool aNotify,
   // Update the display of the placeholder value if needed.
   // We don't need to do this if we're about to initialize the
   // editor, since EnsureEditorInitialized takes care of this.
-  if (!aBeforeEditorInit)
+  if (mUsePlaceholder && !aBeforeEditorInit)
   {
     nsWeakFrame weakFrame(this);
     txtCtrl->SetPlaceholderClass(value.IsEmpty(), aNotify);

layout/forms/nsTextControlFrame.h

@@ -383,6 +383,8 @@ private:
   // eventually) when mFireChangeEventState==true, this is used by nsFileControlFrame.
   PRPackedBool mFireChangeEventState;
   PRPackedBool mInSecureKeyboardInputMode;
+  // Keep track if we have asked a placeholder node creation.
+  PRPackedBool mUsePlaceholder;
 
 #ifdef DEBUG
   PRPackedBool mInEditorInitialization;

layout/generic/crashtests/crashtests.list

@@ -14,7 +14,7 @@ load 310556-1.xhtml
 load 322780-1.xul
 load 323381-1.html
 load 323381-2.html
-asserts(2) asserts-if(gtk2Widget,12) load 323386-1.html # Bug 575011
+asserts(2) asserts-if(gtk2Widget,8) load 323386-1.html # Bug 575011
 load 323389-1.html
 load 323389-2.html
 load 323493-1.html

layout/reftests/forms/placeholder/placeholder-2-textarea.html

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <!-- Test: placeholder has to be used if set via javascript -->
+  <script type="text/javascript">
+    function setPlaceholder()
+    {
+      document.getElementById('p1').placeholder = "my placeholder";
+    }
+    function disableReftestWait()
+    {
+      document.documentElement.className = '';
+    }
+  </script>
+
+  <body onload="setPlaceholder(); disableReftestWait();">
+    <textarea id="p1" placeholder=""></textarea>
+  </body>
+</html>

layout/reftests/forms/placeholder/placeholder-20.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <!-- Test: when focused, placeholder update shouldn't show placeholder. -->
+  <script type="text/javascript">
+    function focusPlaceholder()
+    {
+      document.getElementById('p1').focus();
+    }
+    function setPlaceholder()
+    {
+      document.getElementById('p1').placeholder = 'new placeholder';
+    }
+    function disableReftestWait()
+    {
+      document.documentElement.className = '';
+    }
+  </script>
+
+  <body onload="focusPlaceholder();">
+    <input type="text" id="p1" value="" onfocus="setPlaceholder(); disableReftestWait();">
+  </body>
+</html>

layout/reftests/forms/placeholder/placeholder-21.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <!-- Test: when focused, placeholder update shouldn't show placeholder. -->
+  <script type="text/javascript">
+    function focusPlaceholder()
+    {
+      document.getElementById('p1').focus();
+    }
+    function setPlaceholder()
+    {
+      document.getElementById('p1').removeAttribute('placeholder');
+    }
+    function disableReftestWait()
+    {
+      document.documentElement.className = '';
+    }
+  </script>
+
+  <body onload="focusPlaceholder();">
+    <input type="text" id="p1" value="" placeholder="my placeholder" onfocus="setPlaceholder(); disableReftestWait();">
+  </body>
+</html>

layout/reftests/forms/placeholder/placeholder-22.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <!-- Test: when focused, placeholder update shouldn't show placeholder. -->
+  <script type="text/javascript">
+    function focusPlaceholder()
+    {
+      document.getElementById('p1').focus();
+    }
+    function setPlaceholder()
+    {
+      document.getElementById('p1').placeholder = '';
+    }
+    function disableReftestWait()
+    {
+      document.documentElement.className = '';
+    }
+  </script>
+
+  <body onload="focusPlaceholder();">
+    <input type="text" id="p1" value="" placeholder="my placeholder" onfocus="setPlaceholder(); disableReftestWait();">
+  </body>
+</html>

layout/reftests/forms/placeholder/placeholder-6-textarea.html

@@ -2,6 +2,6 @@
 <html>
   <!-- Test placeholder behavior when textarea is too small -->
   <body>
-    <textarea cols="5" placeholder="my placeholder"></textarea>
+    <textarea cols="5" rows="3" placeholder="my placeholder"></textarea>
   </body>
 </html>

layout/reftests/forms/placeholder/placeholder-7.html

@@ -13,6 +13,6 @@
   </script>
 
   <body onload="focusPlaceholder();">
-    <input type="text" id="p1" value="" placeholder="my placeholder" readonly onfocus="disableReftestWait();">
+    <input type="text" id="p1" value="" placeholder="my placeholder" onfocus="disableReftestWait();">
   </body>
 </html>

layout/reftests/forms/placeholder/placeholder-8.html

@@ -17,6 +17,6 @@
   </script>
 
   <body onload="focusPlaceholder();">
-    <input type="text" id="p1" value="" placeholder="my placeholder" readonly onfocus="setPlaceholder(); disableReftestWait();">
+    <input type="text" id="p1" value="" placeholder="my placeholder" onfocus="setPlaceholder(); disableReftestWait();">
   </body>
 </html>

layout/reftests/forms/placeholder/placeholder-9.html

@@ -17,6 +17,6 @@
   </script>
 
   <body onload="focusPlaceholder();">
-    <input type="text" id="p1" value="my value" placeholder="my placeholder" readonly onfocus="resetValue(); disableReftestWait();">
+    <input type="text" id="p1" value="my value" placeholder="my placeholder" onfocus="resetValue(); disableReftestWait();">
   </body>
 </html>

layout/reftests/forms/placeholder/placeholder-overflow-textarea-ref.html

@@ -8,6 +8,6 @@
   <link rel='stylesheet' type='text/css' href='placeholder-style.css'>
 
   <body>
-    <textarea class="placeholder" cols="5">my placeholder</textarea>
+    <textarea class="placeholder" cols="5" rows="3">my placeholder</textarea>
   </body>
 </html>

layout/reftests/forms/placeholder/placeholder-readonly-focus-ref.html

@@ -1,16 +0,0 @@
-<!DOCTYPE html>
-<html class="reftest-wait">
-  <script type="text/javascript">
-    function focusInput()
-    {
-      document.getElementById('t1').focus();
-    }
-    function disableReftestWait()
-    {
-      document.documentElement.className = '';
-    }
-  </script>
-  <body onload="focusInput();">
-    <input id='t1' type="text" readonly onfocus="disableReftestWait();">
-  </body>
-</html>

layout/reftests/forms/placeholder/reftest.list

@@ -2,14 +2,15 @@
 == placeholder-1-password.html placeholder-visible-ref.html
 == placeholder-1-textarea.html placeholder-visible-textarea-ref.html
 == placeholder-2.html placeholder-visible-ref.html
+== placeholder-2-textarea.html placeholder-visible-textarea-ref.html
 == placeholder-3.html placeholder-overridden-ref.html
 == placeholder-4.html placeholder-overridden-ref.html
 == placeholder-5.html placeholder-visible-ref.html
 == placeholder-6.html placeholder-overflow-ref.html
 == placeholder-6-textarea.html placeholder-overflow-textarea-ref.html
-== placeholder-7.html placeholder-readonly-focus-ref.html
-== placeholder-8.html placeholder-readonly-focus-ref.html
-== placeholder-9.html placeholder-readonly-focus-ref.html
+== placeholder-7.html placeholder-focus-ref.html
+== placeholder-8.html placeholder-focus-ref.html
+== placeholder-9.html placeholder-focus-ref.html
 == placeholder-10.html placeholder-visible-ref.html
 == placeholder-11.html placeholder-visible-ref.html
 == placeholder-12.html placeholder-visible-ref.html
@@ -20,3 +21,6 @@
 == placeholder-17.html placeholder-focus-ref.html
 == placeholder-18.html placeholder-overridden-ref.html
 == placeholder-19.xul  placeholder-overridden-ref.xul
+== placeholder-20.html placeholder-focus-ref.html
+== placeholder-21.html placeholder-focus-ref.html
+== placeholder-22.html placeholder-focus-ref.html