Bug 76353

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

76358

Bug Blocks:

63601, 76354, 76439

Attachments:

Description	Flags
wip. Can not get ShadowRoot JS binding yet	none
ready for review	none
Set Runtime flag in DRT.	none
change ShadowRoot to Node if disabled in Internals.{h,cpp} to fix builds	none
typedef done. dom -> DOM. Build hopufilly fixed	none

Hajime Morrita

Reported 2012-01-15 18:18:58 PST

We're going to add ShadowRoot.idl here.

Attachments
wip. Can not get ShadowRoot JS binding yet (14.83 KB, patch) 2012-01-19 00:17 PST, Hayato Ito	no flags	Details Formatted Diff Diff
ready for review (15.14 KB, patch) 2012-01-19 18:28 PST, Hayato Ito	no flags	Details Formatted Diff Diff
Set Runtime flag in DRT. (19.05 KB, patch) 2012-01-19 22:21 PST, Hayato Ito	no flags	Details Formatted Diff Diff
change ShadowRoot to Node if disabled in Internals.{h,cpp} to fix builds (19.17 KB, patch) 2012-01-19 23:09 PST, Hayato Ito	no flags	Details Formatted Diff Diff
typedef done. dom -> DOM. Build hopufilly fixed (19.24 KB, patch) 2012-01-20 00:37 PST, Hayato Ito	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Hayato Ito

Comment 1 2012-01-19 00:17:50 PST

Created attachment 123085 [details] wip. Can not get ShadowRoot JS binding yet

Hajime Morrita

Comment 2 2012-01-19 01:51:23 PST

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.

Hayato Ito

Comment 3 2012-01-19 18:28:01 PST

Created attachment 123235 [details] ready for review

Early Warning System Bot

Comment 4 2012-01-19 18:51:24 PST

Comment on attachment 123235 [details] ready for review Attachment 123235 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11253659

Hayato Ito

Comment 5 2012-01-19 19:32:55 PST

Let me add APIs for windows.Internals so that setShadowDomEnabled from JS and set the flag to false in default.

Hajime Morrita

Comment 6 2012-01-19 20:12:58 PST

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

Hayato Ito

Comment 7 2012-01-19 22:21:42 PST

Created attachment 123252 [details] Set Runtime flag in DRT.

WebKit Review Bot

Comment 8 2012-01-19 22:24:56 PST

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Hayato Ito

Comment 9 2012-01-19 22:28:01 PST

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.

Hayato Ito

Comment 10 2012-01-19 22:29:16 PST

[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.

Gustavo Noronha (kov)

Comment 11 2012-01-19 22:37:25 PST

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

Early Warning System Bot

Comment 12 2012-01-19 22:38:48 PST

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

Hayato Ito

Comment 13 2012-01-19 22:42:11 PST

Let me try fix build errors on other ports.

Darin Fisher (:fishd, Google)

Comment 14 2012-01-19 23:08:28 PST

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.

Hayato Ito

Comment 15 2012-01-19 23:09:09 PST

Created attachment 123255 [details] change ShadowRoot to Node if disabled in Internals.{h,cpp} to fix builds

Hajime Morrita

Comment 16 2012-01-19 23:45:55 PST

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.

Hayato Ito

Comment 17 2012-01-20 00:37:33 PST

Created attachment 123264 [details] typedef done. dom -> DOM. Build hopufilly fixed

Hayato Ito

Comment 18 2012-01-20 00:39:56 PST

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.

Hajime Morrita

Comment 19 2012-01-20 01:13:33 PST

Comment on attachment 123264 [details] typedef done. dom -> DOM. Build hopufilly fixed Finally it's coming!

WebKit Review Bot

Comment 20 2012-01-20 02:29:30 PST

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>

WebKit Review Bot

Comment 21 2012-01-20 02:29:42 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.