Different Fix for Bug 519933 - only throw TypeError when FEATURE_STRICT_MODE is set

git-svn-id: svn://10.0.0.236/trunk@258763 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/js/rhino/src/org/mozilla/javascript/Context.java

@@ -315,16 +315,6 @@ public class Context
      */
     public static final int FEATURE_ENHANCED_JAVA_ACCESS = 13;
 
-    /**
-     * Enable assignment to properties with only a getter defined.
-     * This was Rhino's standard behaviour until 1.7R2, but for 1.7R2 it will
-     * honor TC39 ES3.1 Draft of 9-Feb-2009, 8.12.4, step 2 and throw a 
-     * TypeError instead. 
-     * By default {@link #hasFeature(int)} returns false.
-     * @since 1.7 Release 2
-     */
-    public static final int FEATURE_WRITE_READONLY_PROPERTIES = 14;
-
     public static final String languageVersionProperty = "language version";
     public static final String errorReporterProperty   = "error reporter";
 

mozilla/js/rhino/src/org/mozilla/javascript/ContextFactory.java

@@ -311,9 +311,6 @@ public class ContextFactory
 
           case Context.FEATURE_ENHANCED_JAVA_ACCESS:
             return false;
-            
-          case Context.FEATURE_WRITE_READONLY_PROPERTIES:
-            return false;
         }
         // It is a bug to call the method with unknown featureIndex
         throw new IllegalArgumentException(String.valueOf(featureIndex));

mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java

@@ -2425,19 +2425,18 @@ public abstract class ScriptableObject implements Scriptable, Serializable,
             slot = getSlot(name, index, SLOT_MODIFY);
         }
         if (slot instanceof GetterSlot) {
-            Object setterObj = ((GetterSlot)slot).setter;
+            final GetterSlot getterSlot = (GetterSlot)slot;
+            final Object setterObj = getterSlot.setter;
             if (setterObj == null) {
-                if (((GetterSlot)slot).getter != null) {
+                if (getterSlot.getter != null) {
-                  if (Context.getContext().hasFeature(Context.FEATURE_WRITE_READONLY_PROPERTIES)) {
+                  if (Context.getContext().hasFeature(Context.FEATURE_STRICT_MODE)) {
-                    // Odd case: Assignment to a property with only a getter 
-                    // defined. The assignment cancels out the getter.
-                    ((GetterSlot)slot).getter = null;
-                  }
-                  else {
                       // Based on TC39 ES3.1 Draft of 9-Feb-2009, 8.12.4, step 2,
                       // we should throw a TypeError in this case.
                       throw ScriptRuntime.typeError1("msg.set.prop.no.setter", name);
                   }
+                  // Odd case: Assignment to a property with only a getter 
+                  // defined. The assignment cancels out the getter.
+                  getterSlot.getter = null;
                 }
             } else {
                 Context cx = Context.getContext();

mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/WriteReadOnlyPropertyTest.java

@@ -0,0 +1,96 @@
+package org.mozilla.javascript.tests;
+
+import java.lang.reflect.Method;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mozilla.javascript.Context;
+import org.mozilla.javascript.ContextAction;
+import org.mozilla.javascript.ContextFactory;
+import org.mozilla.javascript.EcmaError;
+import org.mozilla.javascript.ScriptableObject;
+
+/**
+ * Test that read-only properties can be... set when needed.
+ * This was the standard behavior in Rhino until 1.7R2 but has changed then.
+ * It is needed by HtmlUnit to simulate IE as well as FF2 (but not FF3).
+ * @see <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=519933">Rhino bug 519933</a>
+ * @author Marc Guillemot
+ */
+public class WriteReadOnlyPropertyTest {
+
+	/**
+	 * @throws Exception if the test fails
+	 */
+	@Test
+	public void testWriteReadOnly_accepted() throws Exception {
+		testWriteReadOnly(true);
+	}
+
+	/**
+	 * @throws Exception if the test fails
+	 */
+	@Test
+	public void testWriteReadOnly_throws() throws Exception {
+		try {
+			testWriteReadOnly(false);
+			Assert.fail();
+		}
+		catch (EcmaError e) {
+			Assert.assertTrue(e.getMessage(), e.getMessage().contains("Cannot set property myProp that has only a getter"));
+		}
+	}
+
+	void testWriteReadOnly(final boolean acceptWriteReadOnly) throws Exception {
+		final Method readMethod = Foo.class.getMethod("getMyProp", null);
+		final Foo foo = new Foo("hello");
+        foo.defineProperty("myProp", null, readMethod, null, ScriptableObject.EMPTY);
+
+		final String script = "foo.myProp = 123; foo.myProp";
+
+		final ContextAction action = new ContextAction() {
+			public Object run(final Context cx) {
+
+				final ScriptableObject top = cx.initStandardObjects();
+				ScriptableObject.putProperty(top, "foo", foo);
+				
+				cx.evaluateString(top, script, "script", 0, null);
+				return null;
+			}
+		};
+		
+		final ContextFactory contextFactory = new ContextFactory() {
+			@Override
+			protected boolean hasFeature(final Context cx, final int featureIndex) {
+				if (Context.FEATURE_STRICT_MODE == featureIndex) {
+					return !acceptWriteReadOnly;
+				}
+				return super.hasFeature(cx, featureIndex);
+			}
+		};
+		contextFactory.call(action);
+	}
+
+	/**
+	 * Simple utility allowing to better see the concerned scope while debugging
+	 */
+	static class Foo extends ScriptableObject
+	{
+		final String prop_;
+		Foo(final String label)
+		{
+			prop_ = label;
+		}
+
+		@Override
+		public String getClassName()
+		{
+			return "Foo";
+		}
+		
+		public String getMyProp()
+		{
+			return prop_;
+		}
+	};
+}