126200 2013-12-23 19:15:28 -0800 Handle (numeric) indexed access of DOMStringMap 2022-08-21 16:52:38 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED WORKSFORME https://bugs.webkit.org/show_bug.cgi?id=123890 BlinkMergeCandidate P2 Normal --- 1 rniwa webkit-unassigned ahmad.saleem792 ap benjamin bfulgham cdumez kling oldest_to_newest 962691 0 rniwa 2013-12-23 19:15:28 -0800 Consider merging https://chromium.googlesource.com/chromium/blink/+/54785059007176b07b03810a3f98eb7afa4739b9 As the set of supported property names over DOMStringMap reflects the attributes of an Element prefixed with 'data-', these naturally also include numeric indexes. To allow such index values to be used over DOMStringMap, extend its IDL definition with indexed accessors. Also address an infelicity in comparing attribute names to property names if the property name uses a "-" but isn't followed by an ASCII uppercase letter (e.g. "a-1".) Extended test coverage to check for these variations also. 1892611 1 ahmad.saleem792 2022-08-19 23:59:40 -0700 Not changed: https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DatasetDOMStringMap.cpp#L87 IDL across all browsers: https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/dom/dom_string_map.idl https://hg.mozilla.org/mozilla-central/file/tip/dom/webidl/DOMStringMap.webidl https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DOMStringMap.idl ___ DOMStringMap seems to point to DatasetDOMStringMap: https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DOMStringMap.h but I cannot find link of Chromium change to this: https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DatasetDOMStringMap.h ___ I will leave this for others to comment whether this change is needed any more or not. Thanks! 1892624 2 rniwa 2022-08-20 01:23:00 -0700 We handle numeric index getter just fine. 1892655 3 ap 2022-08-20 09:34:56 -0700 We do, however the Blink patch in question fixes more that that, and many subtests are still failing in WebKit. Would you mind checking if your fix in bug 123890 covers all of these? FAIL testGet('data-r-7', 'r-7') should be true. Was false. FAIL testGet('data-r-7-k', 'r-7K') should be true. Was false. ... FAIL testSet('-foo', 'dummy') should throw SyntaxError: Failed to set the '-foo' property on 'DOMStringMap': '-foo' is not a valid property name.. Threw exception SyntaxError: The string did not match the expected pattern.. ... FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Was false. ... FAIL testDelete('data-r-2', 'r-2') should be true. Was false. FAIL testDelete('data--r-2-', 'R-2-') should be true. Was false. FAIL testDelete('data--r-2r', 'R-2r') should be true. Was false. FAIL testDelete('data--r-2-----r', 'R-2----R') should be true. Was false. 1892708 4 rniwa 2022-08-20 15:15:34 -0700 Yeah, I'm adding those test cases. 1892815 5 ap 2022-08-21 13:04:23 -0700 We are still failing this subtest after the fix: FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Was false. 1892817 6 rniwa 2022-08-21 13:07:58 -0700 (In reply to Alexey Proskuryakov from comment #5) > We are still failing this subtest after the fix: > > FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string > contains invalid characters.. Was false. That test case is bogus. No browser throws exception in that scenario. 1892827 7 ap 2022-08-21 13:40:33 -0700 I see the exception in Chrome 104.0.5112.101 (FAIL because their error message has changed since): FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Threw exception InvalidCharacterError: Failed to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid attribute name.. 1892828 8 ap 2022-08-21 13:41:19 -0700 (and in Firefox too) 1892836 9 rniwa 2022-08-21 16:00:34 -0700 (In reply to Alexey Proskuryakov from comment #7) > I see the exception in Chrome 104.0.5112.101 (FAIL because their error > message has changed since): > > FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string > contains invalid characters.. Threw exception InvalidCharacterError: Failed > to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid > attribute name.. Huh, I guess I got confused while testing. Will address that in a separate bug. 1892838 10 rniwa 2022-08-21 16:05:11 -0700 Tracking in https://bugs.webkit.org/show_bug.cgi?id=244174 1892841 11 rniwa 2022-08-21 16:35:27 -0700 (In reply to Ryosuke Niwa from comment #9) > (In reply to Alexey Proskuryakov from comment #7) > > I see the exception in Chrome 104.0.5112.101 (FAIL because their error > > message has changed since): > > > > FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string > > contains invalid characters.. Threw exception InvalidCharacterError: Failed > > to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid > > attribute name.. > > Huh, I guess I got confused while testing. Will address that in a separate > bug. Hm... actually, I don't get why other browsers are throwing in this case. U+F900 is clearly allowed in NameStartChar: https://www.w3.org/TR/xml/#NT-Name 1892843 12 rniwa 2022-08-21 16:52:38 -0700 This is Blink's implementation: https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/dom/document.cc#L444 I think what happened here is that WebKit implements the latest specification: https://commits.webkit.org/246206@main