It would be nice that we can get an element in nested TreeScope by specifying 'id' and 'owner element'. That enables us to write the following code in tests: function getElementInShadow(path) { var ids = path.split('/'); var element = document.getElementById(ids[0]); for (var i = 1; i < ids.length; ++i) { element = window.internals.getElementByIdInShadowHost(element, ids[i]); } return element; } var inputA = getElementInShadow('shadowHost/shadowHost2/inputA');
I am thinking that we should have window.internals.getElementByIdInTreeScope(shadowRoot, ids[i]) instead of window.internals.getElementByIdInShadowHost(shadowHost, ids[i]) .
Created attachment 100950 [details] first try to know which symbols we should export.
Created attachment 101273 [details] sync to ToT
Comment on attachment 101273 [details] sync to ToT Attachment 101273 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9158050
Created attachment 101289 [details] 2nd try. Add symbols
Created attachment 101297 [details] ready for review
Comment on attachment 101297 [details] ready for review View in context: https://bugs.webkit.org/attachment.cgi?id=101297&action=review > LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:11 > + Please make it clear that this test need internals object and doesn't work interactively. > LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:17 > + element.setAttribute(name, attributes[name]); Don't need braces if the block has single statement. JS in WebKit basically follows our C++ style. > LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:60 > + These helper functions look useful. How about to make it separate js file? > Source/WebCore/testing/Internals.cpp:76 > + ec = INVALID_ACCESS_ERR; If this function is only for ShadowRoot, the name should be getElementByIdInTreeRoot(). > Source/WebCore/testing/Internals.cpp:79 > + return static_cast<ShadowRoot*>(treeScope)->getElementById(id); I guess we have toShadowRoot().
Comment on attachment 101297 [details] ready for review View in context: https://bugs.webkit.org/attachment.cgi?id=101297&action=review >> Source/WebCore/testing/Internals.cpp:76 >> + ec = INVALID_ACCESS_ERR; > > If this function is only for ShadowRoot, the name should be getElementByIdInTreeRoot(). What about getElementByIdIn*Shadow*Root? Tree root sounds like something else—the document, subtree root, whatever.
Comment on attachment 101297 [details] ready for review Thank you for the review. I'll post a updated patch later. View in context: https://bugs.webkit.org/attachment.cgi?id=101297&action=review >> LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:11 >> + > > Please make it clear that this test need internals object and doesn't work interactively. Done. >> LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:17 >> + element.setAttribute(name, attributes[name]); > > Don't need braces if the block has single statement. > JS in WebKit basically follows our C++ style. Done. >> LayoutTests/fast/dom/shadow/get-element-by-id-in-shadow.html:60 >> + > > These helper functions look useful. How about to make it separate js file? Done. I've moved createDom and createShadow functions into a separate js file as 'resources/create-dom.js'. I might move getElemetnInShadow also when we have another layout test which uses this function. >>> Source/WebCore/testing/Internals.cpp:76 >>> + ec = INVALID_ACCESS_ERR; >> >> If this function is only for ShadowRoot, the name should be getElementByIdInTreeRoot(). > > What about getElementByIdIn*Shadow*Root? > > Tree root sounds like something else—the document, subtree root, whatever. Okay. I chose 'getElementByIdInShadowRoot'. >> Source/WebCore/testing/Internals.cpp:79 >> + return static_cast<ShadowRoot*>(treeScope)->getElementById(id); > > I guess we have toShadowRoot(). Oh. When I wrote this patch, there was no header files which defines toShadowRoot. Now we have that in ShadowRoot.h. Done.
Created attachment 101440 [details] revised
ping? Could someone review this?
Committed r92124: <http://trac.webkit.org/changeset/92124>