- Revision
- 96893
- Author
- tk...@chromium.org
- Date
- 2011-10-06 19:44:52 -0700 (Thu, 06 Oct 2011)
Log Message
[JSC binding] Fix inconsistent behavior of DOMStringMap
https://bugs.webkit.org/show_bug.cgi?id=53752
Reviewed by Darin Adler.
Source/WebCore:
The dataset behavior was inconsistent. The get operation handled
data-* attributes first, and the put and delete operations handled
_javascript_ properties first.
Like Firefox and Opera, the put and delete operations should
handle data-* attribute first.
* bindings/js/JSDOMStringMapCustom.cpp:
(WebCore::JSDOMStringMap::deleteProperty):
Handles DOMStringMap first, then returns false if the DOMStringMap makes an error.
(WebCore::JSDOMStringMap::putDelegate): ditto.
LayoutTests:
* fast/dom/dataset-expected.txt:
* fast/dom/script-tests/dataset.js:
- Change the expectation for a case deleting a property of which name can't be a data-* attribute.
This behavior matches to Firefox and Opera.
- Add test cases to check put/get/delete priorities.
* platform/chromium/test_expectations.txt:
V8 binding is not ready for this change.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (96892 => 96893)
--- trunk/LayoutTests/ChangeLog 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/LayoutTests/ChangeLog 2011-10-07 02:44:52 UTC (rev 96893)
@@ -1,3 +1,18 @@
+2011-10-06 Kent Tamura <tk...@chromium.org>
+
+ [JSC binding] Fix inconsistent behavior of DOMStringMap
+ https://bugs.webkit.org/show_bug.cgi?id=53752
+
+ Reviewed by Darin Adler.
+
+ * fast/dom/dataset-expected.txt:
+ * fast/dom/script-tests/dataset.js:
+ - Change the expectation for a case deleting a property of which name can't be a data-* attribute.
+ This behavior matches to Firefox and Opera.
+ - Add test cases to check put/get/delete priorities.
+ * platform/chromium/test_expectations.txt:
+ V8 binding is not ready for this change.
+
2011-10-06 Jer Noble <jer.no...@apple.com>
Some media tests fail when run inside a directory path containing periods.
Modified: trunk/LayoutTests/fast/dom/dataset-expected.txt (96892 => 96893)
--- trunk/LayoutTests/fast/dom/dataset-expected.txt 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/LayoutTests/fast/dom/dataset-expected.txt 2011-10-07 02:44:52 UTC (rev 96893)
@@ -39,12 +39,33 @@
PASS testDelete('data-', '') is true
PASS testDelete('data-à', 'à') is true
-PASS testDelete('dummy', '-foo') threw exception Error: SYNTAX_ERR: DOM Exception 12.
+PASS testDelete('dummy', '-foo') is false
PASS testForIn(['data-foo', 'data-bar', 'data-baz']) is 3
PASS testForIn(['data-foo', 'data-bar', 'dataFoo']) is 2
PASS testForIn(['data-foo', 'data-bar', 'style']) is 2
PASS testForIn(['data-foo', 'data-bar', 'data-']) is 3
+
+Property override:
+PASS Object.prototype.foo = 'on Object'; div.dataset.foo is 'on Object'
+PASS div.dataset['foo'] = 'on dataset'; div.dataset.foo is 'on dataset'
+PASS div.hasAttribute('data-foo') is true
+PASS div.setAttribute('data-foo', 'attr'); div.dataset.foo is 'attr'
+Update the _javascript_ property:
+PASS div.dataset.foo = 'updated'; div.dataset.foo is 'updated'
+PASS div.getAttribute('data-foo') is 'updated'
+PASS div.dataset.Bar = 'on dataset'; div.dataset.Bar is 'on dataset'
+PASS div.hasAttribute('data-Bar') is false
+Make the _javascript_ property empty:
+PASS div.dataset.foo = ''; div.dataset.foo is ''
+PASS div.getAttribute('data-foo') is ''
+Remove the attribute:
+PASS div.removeAttribute('data-foo'); div.dataset.foo is 'on Object'
+Remove the _javascript_ property:
+PASS div.setAttribute('data-foo', 'attr'); delete div.dataset.foo; div.dataset.foo is 'on Object'
+PASS div.hasAttribute('foo') is false
+PASS delete div.dataset.Bar; div.dataset.Bar is undefined.
+
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/dom/script-tests/dataset.js (96892 => 96893)
--- trunk/LayoutTests/fast/dom/script-tests/dataset.js 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/LayoutTests/fast/dom/script-tests/dataset.js 2011-10-07 02:44:52 UTC (rev 96893)
@@ -74,7 +74,7 @@
shouldBeTrue("testDelete('data-\xE0', '\xE0')");
debug("");
-shouldThrow("testDelete('dummy', '-foo')", "'Error: SYNTAX_ERR: DOM Exception 12'");
+shouldBeFalse("testDelete('dummy', '-foo')");
debug("");
function testForIn(array)
@@ -96,4 +96,31 @@
shouldBe("testForIn(['data-foo', 'data-bar', 'style'])", "2");
shouldBe("testForIn(['data-foo', 'data-bar', 'data-'])", "3");
+
+debug("");
+debug("Property override:");
+var div = document.createElement("div");
+// If the Object prorotype already has "foo", dataset doesn't create the
+// corresponding attribute for "foo".
+shouldBe("Object.prototype.foo = 'on Object'; div.dataset.foo", "'on Object'");
+shouldBe("div.dataset['foo'] = 'on dataset'; div.dataset.foo", "'on dataset'");
+shouldBeTrue("div.hasAttribute('data-foo')");
+shouldBe("div.setAttribute('data-foo', 'attr'); div.dataset.foo", "'attr'");
+debug("Update the _javascript_ property:");
+shouldBe("div.dataset.foo = 'updated'; div.dataset.foo", "'updated'");
+shouldBe("div.getAttribute('data-foo')", "'updated'");
+// "Bar" can't be represented as a data- attribute.
+shouldBe("div.dataset.Bar = 'on dataset'; div.dataset.Bar", "'on dataset'");
+shouldBeFalse("div.hasAttribute('data-Bar')");
+debug("Make the _javascript_ property empty:");
+shouldBe("div.dataset.foo = ''; div.dataset.foo", "''");
+shouldBe("div.getAttribute('data-foo')", "''");
+debug("Remove the attribute:");
+shouldBe("div.removeAttribute('data-foo'); div.dataset.foo", "'on Object'");
+debug("Remove the _javascript_ property:");
+shouldBe("div.setAttribute('data-foo', 'attr'); delete div.dataset.foo; div.dataset.foo", "'on Object'");
+shouldBeFalse("div.hasAttribute('foo')");
+shouldBeUndefined("delete div.dataset.Bar; div.dataset.Bar");
+
+debug("");
var successfullyParsed = true;
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (96892 => 96893)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-10-07 02:44:52 UTC (rev 96893)
@@ -3745,6 +3745,9 @@
BUGWK68982 : svg/custom/oversized-pattern-scale.svg = IMAGE PASS
BUGWK68982 : svg/custom/transformed-pattern-clamp-svg-root.svg = IMAGE PASS
+// Need to follow a JSC binding change. See webkit.org/b/53752.
+BUGWK53578 : fast/dom/dataset.html = TEXT
+
// Need rebaselines after r96257
BUGWK62092 MAC : editing/pasteboard/paste-xml.xhtml = TEXT
BUGWK62092 MAC : editing/pasteboard/paste-text-006.html = IMAGE
Modified: trunk/Source/WebCore/ChangeLog (96892 => 96893)
--- trunk/Source/WebCore/ChangeLog 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/Source/WebCore/ChangeLog 2011-10-07 02:44:52 UTC (rev 96893)
@@ -1,3 +1,22 @@
+2011-10-06 Kent Tamura <tk...@chromium.org>
+
+ [JSC binding] Fix inconsistent behavior of DOMStringMap
+ https://bugs.webkit.org/show_bug.cgi?id=53752
+
+ Reviewed by Darin Adler.
+
+ The dataset behavior was inconsistent. The get operation handled
+ data-* attributes first, and the put and delete operations handled
+ _javascript_ properties first.
+
+ Like Firefox and Opera, the put and delete operations should
+ handle data-* attribute first.
+
+ * bindings/js/JSDOMStringMapCustom.cpp:
+ (WebCore::JSDOMStringMap::deleteProperty):
+ Handles DOMStringMap first, then returns false if the DOMStringMap makes an error.
+ (WebCore::JSDOMStringMap::putDelegate): ditto.
+
2011-10-06 Nico Weber <tha...@chromium.org>
[chromium] Let rule_binding use os.execvp() instead of subprocess.call() to spawn fewer processes.
Modified: trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp (96892 => 96893)
--- trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp 2011-10-07 02:43:30 UTC (rev 96892)
+++ trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp 2011-10-07 02:44:52 UTC (rev 96893)
@@ -59,46 +59,24 @@
bool JSDOMStringMap::deleteProperty(ExecState* exec, const Identifier& propertyName)
{
- // Only perform the custom delete if the object doesn't have a native property by this name.
- // Since hasProperty() would end up calling canGetItemsForName() and be fooled, we need to check
- // the native property slots manually.
- PropertySlot slot;
- if (getStaticValueSlot<JSDOMStringMap, Base>(exec, s_info.propHashTable(exec), this, propertyName, slot))
+ AtomicString stringName = identifierToAtomicString(propertyName);
+ if (!m_impl->contains(stringName))
return false;
-
- JSValue prototype = this->prototype();
- if (prototype.isObject() && asObject(prototype)->hasProperty(exec, propertyName))
- return false;
-
ExceptionCode ec = 0;
- m_impl->deleteItem(identifierToString(propertyName), ec);
+ m_impl->deleteItem(stringName, ec);
setDOMException(exec, ec);
-
- return true;
+ return !ec;
}
bool JSDOMStringMap::putDelegate(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot&)
{
- // Only perform the custom put if the object doesn't have a native property by this name.
- // Since hasProperty() would end up calling canGetItemsForName() and be fooled, we need to check
- // the native property slots manually.
- PropertySlot slot;
- if (getStaticValueSlot<JSDOMStringMap, Base>(exec, s_info.propHashTable(exec), this, propertyName, slot))
- return false;
-
- JSValue prototype = this->prototype();
- if (prototype.isObject() && asObject(prototype)->hasProperty(exec, propertyName))
- return false;
-
String stringValue = ustringToString(value.toString(exec));
if (exec->hadException())
- return true;
-
+ return false;
ExceptionCode ec = 0;
impl()->setItem(identifierToString(propertyName), stringValue, ec);
setDOMException(exec, ec);
-
- return true;
+ return !ec;
}
} // namespace WebCore
