⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 96893 in webkit


Ignore:
Timestamp:
Oct 6, 2011, 7:44:52 PM (15 years ago)
Author:
tkent@chromium.org
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.
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r96891 r96893  
     12011-10-06  Kent Tamura  <tkent@chromium.org>
     2
     3        [JSC binding] Fix inconsistent behavior of DOMStringMap
     4        https://bugs.webkit.org/show_bug.cgi?id=53752
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/dom/dataset-expected.txt:
     9        * fast/dom/script-tests/dataset.js:
     10         - Change the expectation for a case deleting a property of which name can't be a data-* attribute.
     11           This behavior matches to Firefox and Opera.
     12         - Add test cases to check put/get/delete priorities.
     13        * platform/chromium/test_expectations.txt:
     14         V8 binding is not ready for this change.
     15
    1162011-10-06  Jer Noble  <jer.noble@apple.com>
    217

  • trunk/LayoutTests/fast/dom/dataset-expected.txt

    r96166 r96893  
    4040PASS testDelete('data-à', 'à') is true
    4141
    42 PASS testDelete('dummy', '-foo') threw exception Error: SYNTAX_ERR: DOM Exception 12.
     42PASS testDelete('dummy', '-foo') is false
    4343
    4444PASS testForIn(['data-foo', 'data-bar', 'data-baz']) is 3
    … …  
    4646PASS testForIn(['data-foo', 'data-bar', 'style']) is 2
    4747PASS testForIn(['data-foo', 'data-bar', 'data-']) is 3
     48
     49Property override:
     50PASS Object.prototype.foo = 'on Object'; div.dataset.foo is 'on Object'
     51PASS div.dataset['foo'] = 'on dataset'; div.dataset.foo is 'on dataset'
     52PASS div.hasAttribute('data-foo') is true
     53PASS div.setAttribute('data-foo', 'attr'); div.dataset.foo is 'attr'
     54Update the JavaScript property:
     55PASS div.dataset.foo = 'updated'; div.dataset.foo is 'updated'
     56PASS div.getAttribute('data-foo') is 'updated'
     57PASS div.dataset.Bar = 'on dataset'; div.dataset.Bar is 'on dataset'
     58PASS div.hasAttribute('data-Bar') is false
     59Make the JavaScript property empty:
     60PASS div.dataset.foo = ''; div.dataset.foo is ''
     61PASS div.getAttribute('data-foo') is ''
     62Remove the attribute:
     63PASS div.removeAttribute('data-foo'); div.dataset.foo is 'on Object'
     64Remove the JavaScript property:
     65PASS div.setAttribute('data-foo', 'attr'); delete div.dataset.foo; div.dataset.foo is 'on Object'
     66PASS div.hasAttribute('foo') is false
     67PASS delete div.dataset.Bar; div.dataset.Bar is undefined.
     68
    4869PASS successfullyParsed is true
    4970

  • trunk/LayoutTests/fast/dom/script-tests/dataset.js

    r96166 r96893  
    7575debug("");
    7676
    77 shouldThrow("testDelete('dummy', '-foo')", "'Error: SYNTAX_ERR: DOM Exception 12'");
     77shouldBeFalse("testDelete('dummy', '-foo')");
    7878debug("");
    7979
    … …  
    9797shouldBe("testForIn(['data-foo', 'data-bar', 'data-'])", "3");
    9898
     99
     100debug("");
     101debug("Property override:");
     102var div = document.createElement("div");
     103// If the Object prorotype already has "foo", dataset doesn't create the
     104// corresponding attribute for "foo".
     105shouldBe("Object.prototype.foo = 'on Object'; div.dataset.foo", "'on Object'");
     106shouldBe("div.dataset['foo'] = 'on dataset'; div.dataset.foo", "'on dataset'");
     107shouldBeTrue("div.hasAttribute('data-foo')");
     108shouldBe("div.setAttribute('data-foo', 'attr'); div.dataset.foo", "'attr'");
     109debug("Update the JavaScript property:");
     110shouldBe("div.dataset.foo = 'updated'; div.dataset.foo", "'updated'");
     111shouldBe("div.getAttribute('data-foo')", "'updated'");
     112// "Bar" can't be represented as a data- attribute.
     113shouldBe("div.dataset.Bar = 'on dataset'; div.dataset.Bar", "'on dataset'");
     114shouldBeFalse("div.hasAttribute('data-Bar')");
     115debug("Make the JavaScript property empty:");
     116shouldBe("div.dataset.foo = ''; div.dataset.foo", "''");
     117shouldBe("div.getAttribute('data-foo')", "''");
     118debug("Remove the attribute:");
     119shouldBe("div.removeAttribute('data-foo'); div.dataset.foo", "'on Object'");
     120debug("Remove the JavaScript property:");
     121shouldBe("div.setAttribute('data-foo', 'attr'); delete div.dataset.foo; div.dataset.foo", "'on Object'");
     122shouldBeFalse("div.hasAttribute('foo')");
     123shouldBeUndefined("delete div.dataset.Bar; div.dataset.Bar");
     124
     125debug("");
    99126var successfullyParsed = true;

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r96888 r96893  
    37463746BUGWK68982 : svg/custom/transformed-pattern-clamp-svg-root.svg = IMAGE PASS
    37473747
     3748// Need to follow a JSC binding change. See webkit.org/b/53752.
     3749BUGWK53578 : fast/dom/dataset.html = TEXT
     3750
    37483751// Need rebaselines after r96257
    37493752BUGWK62092 MAC : editing/pasteboard/paste-xml.xhtml = TEXT

  • trunk/Source/WebCore/ChangeLog

    r96892 r96893  
     12011-10-06  Kent Tamura  <tkent@chromium.org>
     2
     3        [JSC binding] Fix inconsistent behavior of DOMStringMap
     4        https://bugs.webkit.org/show_bug.cgi?id=53752
     5
     6        Reviewed by Darin Adler.
     7
     8        The dataset behavior was inconsistent. The get operation handled
     9        data-* attributes first, and the put and delete operations handled
     10        JavaScript properties first.
     11
     12        Like Firefox and Opera, the put and delete operations should
     13        handle data-* attribute first.
     14
     15        * bindings/js/JSDOMStringMapCustom.cpp:
     16        (WebCore::JSDOMStringMap::deleteProperty):
     17         Handles DOMStringMap first, then returns false if the DOMStringMap makes an error.
     18        (WebCore::JSDOMStringMap::putDelegate): ditto.
     19
    1202011-10-06  Nico Weber  <thakis@chromium.org>
    221

  • trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp

    r95901 r96893  
    6060bool JSDOMStringMap::deleteProperty(ExecState* exec, const Identifier& propertyName)
    6161{
    62     // Only perform the custom delete if the object doesn't have a native property by this name.
    63     // Since hasProperty() would end up calling canGetItemsForName() and be fooled, we need to check
    64     // the native property slots manually.
    65     PropertySlot slot;
    66     if (getStaticValueSlot<JSDOMStringMap, Base>(exec, s_info.propHashTable(exec), this, propertyName, slot))
     62    AtomicString stringName = identifierToAtomicString(propertyName);
     63    if (!m_impl->contains(stringName))
    6764        return false;
    68        
    69     JSValue prototype = this->prototype();
    70     if (prototype.isObject() && asObject(prototype)->hasProperty(exec, propertyName))
    71         return false;
    72 
    7365    ExceptionCode ec = 0;
    74     m_impl->deleteItem(identifierToString(propertyName), ec);
     66    m_impl->deleteItem(stringName, ec);
    7567    setDOMException(exec, ec);
    76 
    77     return true;
     68    return !ec;
    7869}
    7970
    8071bool JSDOMStringMap::putDelegate(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot&)
    8172{
    82     // Only perform the custom put if the object doesn't have a native property by this name.
    83     // Since hasProperty() would end up calling canGetItemsForName() and be fooled, we need to check
    84     // the native property slots manually.
    85     PropertySlot slot;
    86     if (getStaticValueSlot<JSDOMStringMap, Base>(exec, s_info.propHashTable(exec), this, propertyName, slot))
    87         return false;
    88        
    89     JSValue prototype = this->prototype();
    90     if (prototype.isObject() && asObject(prototype)->hasProperty(exec, propertyName))
    91         return false;
    92    
    9373    String stringValue = ustringToString(value.toString(exec));
    9474    if (exec->hadException())
    95         return true;
    96    
     75        return false;
    9776    ExceptionCode ec = 0;
    9877    impl()->setItem(identifierToString(propertyName), stringValue, ec);
    9978    setDOMException(exec, ec);
    100 
    101     return true;
     79    return !ec;
    10280}
    10381
Note: See TracChangeset for help on using the changeset viewer.