Skip installing fields which have no text.
Bug 397924, r+sr+a=sicking
--- a/content/xbl/src/nsXBLBinding.cpp
+++ b/content/xbl/src/nsXBLBinding.cpp
@@ -199,27 +199,33 @@ XBLResolve(JSContext *cx, JSObject *obj,
nsCOMPtr<nsIScriptContext> context = global->GetContext();
if (!context) {
return JS_TRUE;
}
// Now we either resolve or fail
- *objp = origObj;
+ PRBool didInstall;
nsresult rv = field->InstallField(context, origObj,
- protoBinding->DocURI());
+ protoBinding->DocURI(),
+ &didInstall);
if (NS_FAILED(rv)) {
if (!::JS_IsExceptionPending(cx)) {
nsDOMClassInfo::ThrowJSException(cx, rv);
}
return JS_FALSE;
}
+ if (didInstall) {
+ *objp = origObj;
+ }
+ // else we didn't resolve this field after all
+
return JS_TRUE;
}
nsXBLJSClass::nsXBLJSClass(const nsAFlatCString& aClassName)
{
memset(this, 0, sizeof(nsXBLJSClass));
next = prev = static_cast<JSCList*>(this);
name = ToNewCString(aClassName);
--- a/content/xbl/src/nsXBLProtoImplField.cpp
+++ b/content/xbl/src/nsXBLProtoImplField.cpp
@@ -89,59 +89,65 @@ nsXBLProtoImplField::AppendFieldText(con
mFieldText = ToNewUnicode(aText);
mFieldTextLength = aText.Length();
}
}
nsresult
nsXBLProtoImplField::InstallField(nsIScriptContext* aContext,
JSObject* aBoundNode,
- nsIURI* aBindingDocURI) const
+ nsIURI* aBindingDocURI,
+ PRBool* aDidInstall) const
{
NS_PRECONDITION(aBoundNode,
"uh-oh, bound node should NOT be null or bad things will "
"happen");
- jsval result = JSVAL_VOID;
+ *aDidInstall = PR_FALSE;
+
+ if (mFieldTextLength == 0) {
+ return NS_OK;
+ }
+
+ jsval result = JSVAL_NULL;
// EvaluateStringWithValue and JS_DefineUCProperty can both trigger GC, so
// protect |result| here.
nsresult rv;
nsAutoGCRoot root(&result, &rv);
if (NS_FAILED(rv))
return rv;
- if (mFieldTextLength != 0) {
- nsCAutoString uriSpec;
- aBindingDocURI->GetSpec(uriSpec);
+ nsCAutoString uriSpec;
+ aBindingDocURI->GetSpec(uriSpec);
- // compile the literal string
- // XXX Could we produce a better principal here? Should be able
- // to, really!
- PRBool undefined;
- nsCOMPtr<nsIScriptContext> context = aContext;
- rv = context->EvaluateStringWithValue(nsDependentString(mFieldText,
- mFieldTextLength),
- aBoundNode,
- nsnull, uriSpec.get(),
- mLineNumber, nsnull,
- (void*) &result, &undefined);
- if (NS_FAILED(rv))
- return rv;
+ // compile the literal string
+ // XXX Could we produce a better principal here? Should be able
+ // to, really!
+ PRBool undefined;
+ nsCOMPtr<nsIScriptContext> context = aContext;
+ rv = context->EvaluateStringWithValue(nsDependentString(mFieldText,
+ mFieldTextLength),
+ aBoundNode,
+ nsnull, uriSpec.get(),
+ mLineNumber, nsnull,
+ (void*) &result, &undefined);
+ if (NS_FAILED(rv))
+ return rv;
- if (undefined) {
- result = JSVAL_VOID;
- }
+ if (undefined) {
+ result = JSVAL_VOID;
}
// Define the evaluated result as a JS property
nsDependentString name(mName);
JSContext* cx = (JSContext*) aContext->GetNativeContext();
JSAutoRequest ar(cx);
if (!::JS_DefineUCProperty(cx, aBoundNode,
reinterpret_cast<const jschar*>(mName),
name.Length(), result, nsnull, nsnull,
mJSAttributes)) {
return NS_ERROR_OUT_OF_MEMORY;
}
-
+
+ *aDidInstall = PR_TRUE;
return NS_OK;
}
--- a/content/xbl/src/nsXBLProtoImplField.h
+++ b/content/xbl/src/nsXBLProtoImplField.h
@@ -58,18 +58,19 @@ public:
void SetLineNumber(PRUint32 aLineNumber) {
mLineNumber = aLineNumber;
}
nsXBLProtoImplField* GetNext() const { return mNext; }
void SetNext(nsXBLProtoImplField* aNext) { mNext = aNext; }
nsresult InstallField(nsIScriptContext* aContext,
- JSObject* aBoundNode, nsIURI*
- aBindingDocURI) const;
+ JSObject* aBoundNode,
+ nsIURI* aBindingDocURI,
+ PRBool* aDidInstall) const;
const PRUnichar* GetName() const { return mName; }
protected:
nsXBLProtoImplField* mNext;
PRUnichar* mName;
PRUnichar* mFieldText;
PRUint32 mFieldTextLength;
--- a/content/xbl/test/Makefile.in
+++ b/content/xbl/test/Makefile.in
@@ -46,12 +46,13 @@ relativesrcdir = content/xbl/test
include $(DEPTH)/config/autoconf.mk
include $(topsrcdir)/config/rules.mk
_TEST_FILES = \
test_bug296375.xul \
test_bug366770.html \
test_bug371724.xhtml \
test_bug372769.xhtml \
+ test_bug397934.xhtml \
$(NULL)
libs:: $(_TEST_FILES)
$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/xbl/test/test_bug397934.xhtml
@@ -0,0 +1,116 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=397934
+-->
+<head>
+ <title>Test for Bug 397934</title>
+ <script type="text/javascript" src="/MochiKit/packed.js"></script>
+ <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+ <bindings xmlns="http://www.mozilla.org/xbl">
+ <binding id="ancestor">
+ <implementation>
+ <field name="testAncestor">"ancestor"</field>
+ </implementation>
+ </binding>
+ <binding id="test1" extends="#ancestor">
+ <implementation>
+ <constructor>
+ this.storage = window;
+ </constructor>
+ <field name="window"></field>
+ <field name="storage">null</field>
+ <field name="parentNode"></field>
+ <field name="ownerDocument">"ownerDocument"</field>
+ <field name="emptyTest1"></field>
+ <field name="emptyTest1">"empty1"</field>
+ <field name="emptyTest2">"empty2"</field>
+ <field name="emptyTest2"></field>
+ <field name="testAncestor"></field>
+ <field name="testEvalOnce">window.counter++; undefined</field>
+ </implementation>
+ </binding>
+ <binding id="test2" extends="#ancestor">
+ <implementation>
+ <constructor>
+ this.storage = window;
+ </constructor>
+ <field name="window">undefined</field>
+ <field name="storage">null</field>
+ <field name="parentNode">undefined</field>
+ <field name="ownerDocument">"ownerDocument"</field>
+ <field name="emptyTest1">undefined</field>
+ <field name="emptyTest1">"empty1"</field>
+ <field name="emptyTest2">"empty2"</field>
+ <field name="emptyTest2">undefined</field>
+ <field name="testAncestor">undefined</field>
+ </implementation>
+ </binding>
+ </bindings>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=397934">Mozilla Bug 397934</a>
+<p id="display1" style="-moz-binding: url(#test1)"></p>
+<p id="display2" style="-moz-binding: url(#test2)"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+<![CDATA[
+
+/** Test for Bug 397934 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+ var d;
+ d = $("display1");
+ is(d.storage, window,
+ "test1" +
+ ": |window| in the constructor should have resolved to our window");
+ is(d.ownerDocument, "ownerDocument",
+ "test1" + ": Control to test field overriding DOM prop");
+ is(d.parentNode, document.body, "test1" + ": unexpected parent");
+ is(d.emptyTest1, undefined,
+ "test1" + ": First field wins even if undefined");
+ is(typeof(d.emptyTest1), "undefined",
+ "test1" + ": First field wins even if undefined, double-check");
+ is(d.emptyTest2, "empty2",
+ "test1" + ": First field wins");
+ is(d.testAncestor, "ancestor",
+ "test1" + ": Empty field should not override ancestor binding");
+
+ window.counter = 0;
+ d.testEvalOnce;
+ d.testEvalOnce;
+ is(window.counter, 1, "Field should have evaluated once and only once");
+
+ d = $("display2");
+ is(d.storage, undefined,
+ "test2" +
+ ": |window| in the constructor should have resolved to undefined");
+ is(typeof(d.storage), "undefined",
+ "test2" +
+ ": |window| in the constructor should really have resolved to undefined");
+ is(d.ownerDocument, "ownerDocument",
+ "test2" + ": Control to test field overriding DOM prop");
+ is(d.parentNode, undefined, "test2" + ": unexpected parent");
+ is(typeof(d.parentNode), "undefined", "test2" + ": unexpected parent for real");
+ is(d.emptyTest1, undefined,
+ "test2" + ": First field wins even if undefined");
+ is(typeof(d.emptyTest1), "undefined",
+ "test2" + ": First field wins even if undefined, double-check");
+ is(d.emptyTest2, "empty2",
+ "test2" + ": First field wins");
+ is(d.testAncestor, undefined,
+ "test2" + ": Undefined field should override ancestor binding");
+ is(typeof(d.testAncestor), "undefined",
+ "test2" + ": Undefined field should really override ancestor binding");
+});
+addLoadEvent(SimpleTest.finish);
+
+]]>
+</script>
+</pre>
+</body>
+</html>
+