Bug 477700. Push a null JSContext around editor init, so it'll work no matter what JS is on the stack. r+sr=smaug

content/base/public/nsContentUtils.h

@@ -1476,12 +1476,25 @@ public:
 
   // Returns PR_FALSE if something erroneous happened.
   PRBool Push(nsPIDOMEventTarget *aCurrentTarget);
+  // If a null JSContext is passed to Push(), that will cause no
+  // Push() to happen and an error to be returned.
   PRBool Push(JSContext *cx);
+  // Explicitly push a null JSContext on the the stack
+  PRBool PushNull();
+
+  // Pop() will be a no-op if Push() or PushNull() fail
   void Pop();
 
 private:
+  // Combined code for PushNull() and Push(JSContext*)
+  PRBool DoPush(JSContext* cx);
+
   nsCOMPtr<nsIScriptContext> mScx;
   PRBool mScriptIsRunning;
+  PRBool mPushedSomething;
+#ifdef DEBUG
+  JSContext* mPushedContext;
+#endif
 };
 
 class nsAutoGCRoot {

content/base/src/nsContentUtils.cpp

@@ -2656,7 +2656,8 @@ nsContentUtils::GetEventArgNames(PRInt32 aNameSpaceID,
 }
 
 nsCxPusher::nsCxPusher()
-    : mScriptIsRunning(PR_FALSE)
+    : mScriptIsRunning(PR_FALSE),
+      mPushedSomething(PR_FALSE)
 {
 }
 
@@ -2701,7 +2702,7 @@ IsContextOnStack(nsIJSContextStack *aStack, JSContext *aContext)
 PRBool
 nsCxPusher::Push(nsPIDOMEventTarget *aCurrentTarget)
 {
-  if (mScx) {
+  if (mPushedSomething) {
     NS_ERROR("Whaaa! No double pushing with nsCxPusher::Push()!");
 
     return PR_FALSE;
@@ -2729,39 +2730,68 @@ nsCxPusher::Push(nsPIDOMEventTarget *aCurrentTarget)
 PRBool
 nsCxPusher::Push(JSContext *cx)
 {
-  if (mScx) {
+  if (mPushedSomething) {
     NS_ERROR("Whaaa! No double pushing with nsCxPusher::Push()!");
 
     return PR_FALSE;
   }
 
-  if (cx) {
+  if (!cx) {
-    mScx = GetScriptContextFromJSContext(cx);
+    return PR_FALSE;
-    if (!mScx) {
-      // Should probably return PR_FALSE. See bug 416916.
-      return PR_TRUE;
-    }
-
-    nsIThreadJSContextStack* stack = nsContentUtils::ThreadJSContextStack();
-    if (stack) {
-      if (IsContextOnStack(stack, cx)) {
-        // If the context is on the stack, that means that a script
-        // is running at the moment in the context.
-        mScriptIsRunning = PR_TRUE;
-      }
-
-      stack->Push(cx);
-    }
   }
+
+  // Hold a strong ref to the nsIScriptContext, just in case
+  // XXXbz do we really need to?  If we don't get one of these in Pop(), is
+  // that really a problem?  Or do we need to do this to effectively root |cx|?
+  mScx = GetScriptContextFromJSContext(cx);
+  if (!mScx) {
+    // Should probably return PR_FALSE. See bug 416916.
+    return PR_TRUE;
+  }
+
+  return DoPush(cx);
+}
+
+PRBool
+nsCxPusher::DoPush(JSContext* cx)
+{
+  nsIThreadJSContextStack* stack = nsContentUtils::ThreadJSContextStack();
+  if (!stack) {
+    return PR_TRUE;
+  }
+
+  if (cx && IsContextOnStack(stack, cx)) {
+    // If the context is on the stack, that means that a script
+    // is running at the moment in the context.
+    mScriptIsRunning = PR_TRUE;
+  }
+
+  if (NS_FAILED(stack->Push(cx))) {
+    mScriptIsRunning = PR_FALSE;
+    mScx = nsnull;
+    return PR_FALSE;
+  }
+
+  mPushedSomething = PR_TRUE;
+#ifdef DEBUG
+  mPushedContext = cx;
+#endif
   return PR_TRUE;
 }
 
+PRBool
+nsCxPusher::PushNull()
+{
+  return DoPush(nsnull);
+}
+
 void
 nsCxPusher::Pop()
 {
   nsIThreadJSContextStack* stack = nsContentUtils::ThreadJSContextStack();
-  if (!mScx || !stack) {
+  if (!mPushedSomething || !stack) {
     mScx = nsnull;
+    mPushedSomething = PR_FALSE;
 
     NS_ASSERTION(!mScriptIsRunning, "Huh, this can't be happening, "
                  "mScriptIsRunning can't be set here!");
@@ -2772,7 +2802,9 @@ nsCxPusher::Pop()
   JSContext *unused;
   stack->Pop(&unused);
 
-  if (!mScriptIsRunning) {
+  NS_ASSERTION(unused == mPushedContext, "Unexpected context popped");
+
+  if (!mScriptIsRunning && mScx) {
     // No JS is running in the context, but executing the event handler might have
     // caused some JS to run. Tell the script context that it's done.
 
@@ -2781,6 +2813,7 @@ nsCxPusher::Pop()
 
   mScx = nsnull;
   mScriptIsRunning = PR_FALSE;
+  mPushedSomething = PR_FALSE;
 }
 
 static const char gPropertiesFiles[nsContentUtils::PropertiesFile_COUNT][56] = {

layout/forms/nsTextControlFrame.cpp

@@ -1386,6 +1386,11 @@ nsTextControlFrame::CalcIntrinsicSize(nsIRenderingContext* aRenderingContext,
 void
 nsTextControlFrame::DelayedEditorInit()
 {
+  // Time to mess with our security context... See comments in GetValue()
+  // for why this is needed.
+  nsCxPusher pusher;
+  pusher.PushNull();
+
   InitEditor();
   // Notify the text listener we have focus and setup the caret etc (bug 446663).
   if (IsFocusedContent(PresContext(), GetContent())) {
@@ -2569,17 +2574,12 @@ nsTextControlFrame::GetValue(nsAString& aValue, PRBool aIgnoreWrap) const
     // XXXbz if we could just get the textContent of our anonymous content (eg
     // if plaintext editor didn't create <br> nodes all over), we wouldn't need
     // this.
-    nsCOMPtr<nsIJSContextStack> stack =
+    { /* Scope for context pusher */
-      do_GetService("@mozilla.org/js/xpc/ContextStack;1");
+      nsCxPusher pusher;
-    PRBool pushed = stack && NS_SUCCEEDED(stack->Push(nsnull));
+      pusher.PushNull();
       
-    rv = mEditor->OutputToString(NS_LITERAL_STRING("text/plain"), flags,
+      rv = mEditor->OutputToString(NS_LITERAL_STRING("text/plain"), flags,
-                                 aValue);
+                                   aValue);
-
-    if (pushed) {
-      JSContext* cx;
-      stack->Pop(&cx);
-      NS_ASSERTION(!cx, "Unexpected JSContext popped!");
     }
   }
   else
@@ -2647,77 +2647,69 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       NS_ENSURE_SUCCESS(rv, rv);
       NS_ENSURE_STATE(domDoc);
 
+      PRBool outerTransaction;
       // Time to mess with our security context... See comments in GetValue()
       // for why this is needed.  Note that we have to do this up here, because
       // otherwise SelectAll() will fail.
-      nsCOMPtr<nsIJSContextStack> stack =
+      { /* Scope for context pusher */
-        do_GetService("@mozilla.org/js/xpc/ContextStack;1");
+        nsCxPusher pusher;
-      PRBool pushed = stack && NS_SUCCEEDED(stack->Push(nsnull));
+        pusher.PushNull();
 
-      nsCOMPtr<nsISelection> domSel;
+        nsCOMPtr<nsISelection> domSel;
-      nsCOMPtr<nsISelectionPrivate> selPriv;
+        nsCOMPtr<nsISelectionPrivate> selPriv;
-      mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL, getter_AddRefs(domSel));
+        mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
-      if (domSel)
+                              getter_AddRefs(domSel));
-      {
+        if (domSel)
-        selPriv = do_QueryInterface(domSel);
+        {
-        if (selPriv)
+          selPriv = do_QueryInterface(domSel);
-          selPriv->StartBatchChanges();
+          if (selPriv)
-      }
+            selPriv->StartBatchChanges();
-
-      nsCOMPtr<nsISelectionController> kungFuDeathGrip = mSelCon;
-      mSelCon->SelectAll();
-      nsCOMPtr<nsIPlaintextEditor> plaintextEditor = do_QueryInterface(editor);
-      if (!plaintextEditor || !weakFrame.IsAlive()) {
-        NS_WARNING("Somehow not a plaintext editor?");
-        if (pushed) {
-          JSContext* cx;
-          stack->Pop(&cx);
-          NS_ASSERTION(!cx, "Unexpected JSContext popped!");
         }
-        return NS_ERROR_FAILURE;
-      }
 
-      // Since this code does not handle user-generated changes to the text,
+        nsCOMPtr<nsISelectionController> kungFuDeathGrip = mSelCon;
-      // make sure we don't fire oninput when the editor notifies us.
+        mSelCon->SelectAll();
-      // (mNotifyOnInput must be reset before we return).
+        nsCOMPtr<nsIPlaintextEditor> plaintextEditor = do_QueryInterface(editor);
+        if (!plaintextEditor || !weakFrame.IsAlive()) {
+          NS_WARNING("Somehow not a plaintext editor?");
+          return NS_ERROR_FAILURE;
+        }
 
-      // To protect against a reentrant call to SetValue, we check whether
+        // Since this code does not handle user-generated changes to the text,
-      // another SetValue is already happening for this frame.  If it is,
+        // make sure we don't fire oninput when the editor notifies us.
-      // we must wait until we unwind to re-enable oninput events.
+        // (mNotifyOnInput must be reset before we return).
-      PRBool outerTransaction = mNotifyOnInput;
-      if (outerTransaction)
-        mNotifyOnInput = PR_FALSE;
 
-      // get the flags, remove readonly and disabled, set the value,
+        // To protect against a reentrant call to SetValue, we check whether
-      // restore flags
+        // another SetValue is already happening for this frame.  If it is,
-      PRUint32 flags, savedFlags;
+        // we must wait until we unwind to re-enable oninput events.
-      editor->GetFlags(&savedFlags);
+        outerTransaction = mNotifyOnInput;
-      flags = savedFlags;
+        if (outerTransaction)
-      flags &= ~(nsIPlaintextEditor::eEditorDisabledMask);
+          mNotifyOnInput = PR_FALSE;
-      flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
-      editor->SetFlags(flags);
 
-      // Also don't enforce max-length here
+        // get the flags, remove readonly and disabled, set the value,
-      PRInt32 savedMaxLength;
+        // restore flags
-      plaintextEditor->GetMaxTextLength(&savedMaxLength);
+        PRUint32 flags, savedFlags;
-      plaintextEditor->SetMaxTextLength(-1);
+        editor->GetFlags(&savedFlags);
+        flags = savedFlags;
+        flags &= ~(nsIPlaintextEditor::eEditorDisabledMask);
+        flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
+        editor->SetFlags(flags);
 
-      if (currentValue.Length() < 1)
+        // Also don't enforce max-length here
-        editor->DeleteSelection(nsIEditor::eNone);
+        PRInt32 savedMaxLength;
-      else {
+        plaintextEditor->GetMaxTextLength(&savedMaxLength);
-        if (plaintextEditor)
+        plaintextEditor->SetMaxTextLength(-1);
-          plaintextEditor->InsertText(currentValue);
-      }
 
-      plaintextEditor->SetMaxTextLength(savedMaxLength);
+        if (currentValue.Length() < 1)
-      editor->SetFlags(savedFlags);
+          editor->DeleteSelection(nsIEditor::eNone);
-      if (selPriv)
+        else {
-        selPriv->EndBatchChanges();
+          if (plaintextEditor)
+            plaintextEditor->InsertText(currentValue);
+        }
 
-      if (pushed) {
+        plaintextEditor->SetMaxTextLength(savedMaxLength);
-        JSContext* cx;
+        editor->SetFlags(savedFlags);
-        stack->Pop(&cx);
+        if (selPriv)
-        NS_ASSERTION(!cx, "Unexpected JSContext popped!");
+          selPriv->EndBatchChanges();
       }
 
       NS_ENSURE_STATE(weakFrame.IsAlive());

layout/forms/test/Makefile.in

@@ -44,13 +44,17 @@ relativesrcdir  = layout/forms/test
 include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
-_TEST_FILES =	test_bug345267.html \
+_TEST_FILES =	test_bug287446.html \
+		bug287446_subframe.html \
+		test_bug345267.html \
 		test_bug348236.html \
 		test_bug402198.html \
 		test_bug411236.html \
 		test_bug446663.html \
 		test_bug476308.html \
 		test_bug477531.html \
+		test_bug477700.html \
+		bug477700_subframe.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)

layout/forms/test/bug287446_subframe.html

@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+    <script>
+      function doIs(arg1, arg2, arg3) {
+        window.parent.postMessage("t " + encodeURIComponent(arg1) + " " +
+                                  encodeURIComponent(arg2) + " " +
+                                  encodeURIComponent(arg3), "*");
+      }
+
+      function $(arg) { return document.getElementById(arg); }
+
+      window.addEventListener("message",
+        function(evt) {
+          var t = $("target");
+          if (evt.data == "start") {
+            doIs(t.value, "Test", "Shouldn't have lost our initial value");
+            t.focus();
+            sendString("Foo");
+            doIs(t.value, "TestFoo", "Typing should work");
+            window.parent.postMessage("c", "*");
+          } else {
+            doIs(evt.data, "continue", "Unexpected message");
+            doIs(t.value, "TestFoo", "Shouldn't have lost our typed value");
+            sendString("Bar");
+            doIs(t.value, "TestFooBar", "Typing should still work");
+            window.parent.postMessage("f", "*");
+          }
+        },
+        "false");
+      
+    </script>
+  </head>
+  <body>
+    <input id="target" value="Test">
+  </body>
+</html>
+           

layout/forms/test/bug477700_subframe.html

@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+    <script>
+      function doIs(arg1, arg2, arg3) {
+        window.parent.postMessage("t " + encodeURIComponent(arg1) + " " +
+                                  encodeURIComponent(arg2) + " " +
+                                  encodeURIComponent(arg3), "*");
+      }
+
+      function $(arg) { return document.getElementById(arg); }
+
+      window.addEventListener("message",
+        function(evt) {
+          doIs(evt.data, "start", "Unexpected message");
+          sendString("Test");
+          var t = $("target");
+          doIs(t.value, "Test", "Typing should work");
+          (function() {
+            netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+            t.QueryInterface(Components.interfaces.nsIDOMNSEditableElement).editor.undo(1);
+          })()
+          doIs(t.value, "", "Undo should work");
+          (function() {
+            netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+            t.QueryInterface(Components.interfaces.nsIDOMNSEditableElement).editor.redo(1);
+          })()
+          doIs(t.value, "Test", "Redo should work");
+          window.parent.postMessage("f", "*");
+        },
+        "false");
+      
+    </script>
+  </head>
+  <body>
+    <input id="target">
+  </body>
+</html>
+           

layout/forms/test/test_bug287446.html

@@ -0,0 +1,77 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=287446
+-->
+<head>
+  <title>Test for Bug 287446</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.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=287446">Mozilla Bug 287446</a>
+<p id="display">
+  <iframe id="i"
+          src="http://example.com/tests/layout/forms/test/bug287446_subframe.html"></iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 287446 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+  isnot(window.location.host, "example.com", "test is not testing cross-site");
+  var accessed = false;
+  try {
+    $("i").contentDocument.documentElement;
+    accessed = true;
+  } catch(e) {}
+  is(accessed, false, "Shouldn't be able to access cross-site");
+
+  $("i").style.display = "none";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "none",
+     "toggling display failed");
+  $("i").style.display = "";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "inline",
+     "toggling display back failed");
+
+  $("i").contentWindow.postMessage("start", "*");
+});
+
+function continueTest() {
+  dump('aaa');
+  $("i").style.display = "none";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "none",
+     "toggling display second time failed");
+  $("i").style.display = "";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "inline",
+     "toggling display back second time failed");
+
+$("i").contentWindow.postMessage("continue", "*");
+}
+
+window.addEventListener("message",
+  function(evt) {
+    var arr = evt.data.split(/ /).map(decodeURIComponent);
+    if (arr[0] == 't') {
+      is(arr[1], arr[2], arr[3]);
+    } else if (arr[0] == 'c') {
+      continueTest();
+    } else if (arr[0] == 'f') {
+      SimpleTest.finish();
+    }
+  },
+  false);
+
+</script>
+</pre>
+</body>
+</html>

layout/forms/test/test_bug477700.html

@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=477700
+-->
+<head>
+  <title>Test for Bug 477700</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.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=477700">Mozilla Bug 477700</a>
+<p id="display">
+  <iframe id="i"
+          src="http://example.com/tests/layout/forms/test/bug477700_subframe.html"></iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 477700 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+  isnot(window.location.host, "example.com", "test is not testing cross-site");
+  var accessed = false;
+  try {
+    $("i").contentDocument.documentElement;
+    accessed = true;
+  } catch(e) {}
+  is(accessed, false, "Shouldn't be able to access cross-site");
+
+  $("i").style.display = "none";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "none",
+     "toggling display failed");
+  $("i").style.display = "";
+  document.body.offsetWidth;
+  is(document.defaultView.getComputedStyle($("i"), "").display, "inline",
+     "toggling display back failed");
+
+  $("i").contentWindow.postMessage("start", "*");
+});
+
+window.addEventListener("message",
+  function(evt) {
+    var arr = evt.data.split(/ /).map(decodeURIComponent);
+    if (arr[0] == 't') {
+      is(arr[1], arr[2], arr[3]);
+    } else if (arr[0] == 'f') {
+      SimpleTest.finish();
+    }
+  },
+  false);
+
+</script>
+</pre>
+</body>
+</html>