| Summary: | [Shadow DOM] ShadowRoot should have Web facing API. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Hajime Morrita <morrita> |
| Component: | DOM | Assignee: | Hajime Morrita <morrita> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | abarth, fishd, gustavo, hayato, ojan, shinyak, tkent, webkit.review.bot, xan.lopez |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | 76358 | ||
| Bug Blocks: | 63601, 76354, 76439 | ||
| Attachments: | |||
|
Description
Hajime Morrita
2012-01-15 18:18:58 PST
Created attachment 123085 [details]
wip. Can not get ShadowRoot JS binding yet
Comment on attachment 123085 [details] wip. Can not get ShadowRoot JS binding yet View in context: https://bugs.webkit.org/attachment.cgi?id=123085&action=review > Source/WebCore/testing/Internals.idl:33 > Node ensureShadowRoot(in Element host) raises (DOMException); We need to align the returned type between ensureShadowRoot() and shadowRoot(). Without that, we different type of wrapper object is created for each method call which breaks our assumption. For example, ensureShadowRoot() create Node wrapper and implicitly cache it, then shadowRoot() returns the cached Node wrapper regardless we are expecting a ShadowRoot wrapper. Created attachment 123235 [details]
ready for review
Comment on attachment 123235 [details] ready for review Attachment 123235 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11253659 Let me add APIs for windows.Internals so that setShadowDomEnabled from JS and set the flag to false in default. Comment on attachment 123235 [details] ready for review View in context: https://bugs.webkit.org/attachment.cgi?id=123235&action=review > Source/WebCore/dom/ShadowRoot.idl:33 > + readonly attribute Element shadowHost; This should be named "host" http://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#shadow-root-object Created attachment 123252 [details]
Set Runtime flag in DRT.
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API. Done. Instead of adding custom binding, I simply added host() member function to C++ class of ShadowRoot. I am not sure which is widely used technique. If there is a convention, please let me know. Other changes from the previous patch: - Runtime flag for ShadowDOM is not false on default. DRT sets it true. [Correction]
> - Runtime flag for ShadowDOM is not false on default. DRT sets it true.
- Runtime flag for ShadowDOM becomes false on default. DRT sets it to true.
Comment on attachment 123252 [details] Set Runtime flag in DRT. Attachment 123252 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11127340 Comment on attachment 123252 [details] Set Runtime flag in DRT. Attachment 123252 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11247319 Let me try fix build errors on other ports. Comment on attachment 123252 [details] Set Runtime flag in DRT. View in context: https://bugs.webkit.org/attachment.cgi?id=123252&action=review > Source/WebKit/chromium/public/WebRuntimeFeatures.h:124 > + WEBKIT_EXPORT static void enableShadowDom(bool); I think WebKit style is to capitalize acronyms unless they appear at the beginning of a function or variable name. So, enableShadowDOM and isShadowDOMEnabled would be correct style. Created attachment 123255 [details]
change ShadowRoot to Node if disabled in Internals.{h,cpp} to fix builds
Comment on attachment 123255 [details] change ShadowRoot to Node if disabled in Internals.{h,cpp} to fix builds View in context: https://bugs.webkit.org/attachment.cgi?id=123255&action=review > Source/WebCore/testing/Internals.h:59 > + ShadowRoot* ensureShadowRoot(Element* host, ExceptionCode&); Could you hide these type difference by typedef-ing them? That would minimize the number of ifdefs. Created attachment 123264 [details]
typedef done. dom -> DOM. Build hopufilly fixed
Thank you for the review. (In reply to comment #14) > I think WebKit style is to capitalize acronyms unless they appear at the beginning of a function or variable name. > So, enableShadowDOM and isShadowDOMEnabled would be correct style. Done. I've renamed the names. (In reply to comment #16) > Could you hide these type difference by typedef-ing them? > That would minimize the number of ifdefs. Done. Comment on attachment 123264 [details]
typedef done. dom -> DOM. Build hopufilly fixed
Finally it's coming!
Comment on attachment 123264 [details] typedef done. dom -> DOM. Build hopufilly fixed Clearing flags on attachment: 123264 Committed r105500: <http://trac.webkit.org/changeset/105500> All reviewed patches have been landed. Closing bug. |