HTML5 specifies a mechanism for custom HTML attributes (prefixed with data-) and DOM APIs to easily access the value of such attributes. http://dev.w3.org/html5/spec/Overview.html#embedding-custom-non-visible-data WebKit should support the .dataset DOM property such that one could easily and cleanly mark HTML elements with custom attributes.
rdar://problem/8126069
Created attachment 60068 [details] [WIP] Patch I am throwing up my WIP patch so that I don't loose track of it. Things that should be addressed before landing: - The implementation of item() and contains() are unnecessarily slow and allocate an unnecessary String for the propertyNameMatchesAttributeName() check. - I am not sure the behavior for an attribute called "data-" (nothing following the hyphen) is correct. (With this patch, for <div data-="value">, div.dataset[""] == "value"). This should also be included in the test. - I have not included build system changes for all the ports.
Created attachment 60087 [details] Updated patch This is now ready for review. propertyNameMatchesAttributeName() is still unnecessarily dumb, but this can be fixed in a follow up patch since it is merely an optimization.
Attachment 60087 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3313972
Attachment 60087 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3381024
Attachment 60087 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3372044
(In reply to comment #2) > - I am not sure the behavior for an attribute called "data-" (nothing following the hyphen) is correct. (With this patch, for <div data-="value">, div.dataset[""] == "value"). This should also be included in the test. "Custom data attributes" (the ones that should be included in .dataset) must have at least one character after the hyphen. First paragraph of the relevant section of the spec, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data @data- isn't a valid custom data attribute, and so shouldn't be included. It's just a random unstandardized attribute.
(In reply to comment #6) > Attachment 60087 [details] did not build on chromium: > Build output: http://webkit-commit-queue.appspot.com/results/3372044 Nate, can you help Sam with the V8 bindings? This is cool stuff!
(In reply to comment #7) > (In reply to comment #2) > > - I am not sure the behavior for an attribute called "data-" (nothing following the hyphen) is correct. (With this patch, for <div data-="value">, div.dataset[""] == "value"). This should also be included in the test. > > "Custom data attributes" (the ones that should be included in .dataset) must have at least one character after the hyphen. First paragraph of the relevant section of the spec, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data > > @data- isn't a valid custom data attribute, and so shouldn't be included. It's just a random unstandardized attribute. Interesting. In the section on "The algorithm for getting the list of name-value pairs" just below that it states "For each content attribute on the element whose first five characters are the string "data-" and whose remaining characters (if any)".
(In reply to comment #9) > (In reply to comment #7) > > (In reply to comment #2) > > > - I am not sure the behavior for an attribute called "data-" (nothing following the hyphen) is correct. (With this patch, for <div data-="value">, div.dataset[""] == "value"). This should also be included in the test. > > > > "Custom data attributes" (the ones that should be included in .dataset) must have at least one character after the hyphen. First paragraph of the relevant section of the spec, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data > > > > @data- isn't a valid custom data attribute, and so shouldn't be included. It's just a random unstandardized attribute. > > Interesting. In the section on "The algorithm for getting the list of name-value pairs" just below that it states "For each content attribute on the element whose first five characters are the string "data-" and whose remaining characters (if any)". Ah, that's a spec bug. I'll get it corrected one way or the other.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #7) > > > (In reply to comment #2) > > > > - I am not sure the behavior for an attribute called "data-" (nothing following the hyphen) is correct. (With this patch, for <div data-="value">, div.dataset[""] == "value"). This should also be included in the test. > > > > > > "Custom data attributes" (the ones that should be included in .dataset) must have at least one character after the hyphen. First paragraph of the relevant section of the spec, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data > > > > > > @data- isn't a valid custom data attribute, and so shouldn't be included. It's just a random unstandardized attribute. > > > > Interesting. In the section on "The algorithm for getting the list of name-value pairs" just below that it states "For each content attribute on the element whose first five characters are the string "data-" and whose remaining characters (if any)". > > Ah, that's a spec bug. I'll get it corrected one way or the other. Thanks Tab.
It's not a spec bug — don't confuse authoring conformance criteria with implementation conformance criteria. :-) Only parts of the spec that say what implementors must do are relevant here. Parts of the spec that say what is valid are irrelevant to implementors.
(In reply to comment #12) > It's not a spec bug — don't confuse authoring conformance criteria with implementation conformance criteria. :-) Only parts of the spec that say what implementors must do are relevant here. Parts of the spec that say what is valid are irrelevant to implementors. While it's not a direct contradiction, it seems kind of pointless to support "data-"[sic] in the implementation when it's not valid. Since this is a new feature, we could make what is supported closer to what is valid, unless there is a specific reason for this to be a special case.
On the other hand, I guess the "XML-compatible" and "no capital letters" constraints on validity are not ones that make sense to enforce in the implementation.
(In reply to comment #14) > On the other hand, I guess the "XML-compatible" and "no capital letters" constraints on validity are not ones that make sense to enforce in the implementation. The spec *does* mandate enforcing the "no capital letters" thing, though. I can understand if enforcing XML-compatibility is overkill, but if we're enforcing no-capital-letters, surely we can enforce must-have-at-least-one-letter too, and bring author conformance and impl conformance closer together. I don't see a good reason for this gap, seeing as there's no legacy content forcing us to process "data-" attributes even though they're invalid.
I'm happy to make it getting or setting the empty string throw a SYNTAX_ERR if you want, the only reason I hadn't done that was that it was easier to not bother. However, the total time now spent on discussing this exceeds the time it would have saved, so nevermind! :-) If you decide that's the way to go, just implement it that way, and mail the list saying you've done so and asking for the spec to change. I'll just stick a new step at the top of each of the three algorithms that raises SYNTAX_ERR if the input key is "". The capital letter thing is enforced because of the mapping of capital letters to hyphen-lowercase, which we added recently based on author feedback.
Created attachment 60143 [details] Patch 3 - More tests
Landed in r62215. I left it with the spec'd behavior of allowing dataset[""] to match data-.
http://trac.webkit.org/changeset/62215 might have broken Qt Linux Release
(In reply to comment #19) > http://trac.webkit.org/changeset/62215 might have broken Qt Linux Release 06/30/10 16:15:15 (6 hours ago) This appears to have broken fast/dom/Window/window-postmessage-clone.html on every platform. http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r62215%20(16637)/fast/dom/Window/window-postmessage-clone-pretty-diff.html
I think this broke a post-message test? http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r62222%20(16640)/fast/dom/Window/window-postmessage-clone-diffs.txt --- /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug-tests/build/layout-test-results/fast/dom/Window/window-postmessage-clone-expected.txt 2010-06-30 18:27:21.000000000 -0700 +++ /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug-tests/build/layout-test-results/fast/dom/Window/window-postmessage-clone-actual.txt 2010-06-30 18:27:21.000000000 -0700 @@ -2,7 +2,7 @@ PASS: 'postMessage(cyclicObject)' threw TypeError: Cannot post cyclic structures. PASS: 'postMessage(cyclicArray)' threw TypeError: Cannot post cyclic structures. PASS: 'postMessage(reallyDeepArray)' threw RangeError: Maximum call stack size exceeded. -PASS: 'postMessage(window)' threw TypeError: Cannot post cyclic structures. +PASS: 'postMessage(window)' threw TypeError: Type error PASS: eventData is null of type object PASS: eventData is undefined of type undefined PASS: eventData is 1 of type number
(Oops, Adam had the same thought it seems.)
Landed new expectations in http://trac.webkit.org/changeset/62224 Insert nastygram about building and testing your patch before landing or at least watching the bots or listening to SheriffBot. Please consider using the commit-queue to land your patches if you don't have time to build and run all the tests.