WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
78875
[v8] v8 doesn't assume to do 'new WebKitShadowRoot(host)'.
https://bugs.webkit.org/show_bug.cgi?id=78875
Summary
[v8] v8 doesn't assume to do 'new WebKitShadowRoot(host)'.
Shinya Kawanaka
Reported
2012-02-16 21:50:11 PST
new WebKitShadowRoot(host) and internals.ensureShadowRoot(host) returns a different object now. These should be the same, however they return different objects, which contain the same ShadowRoot object though.
Attachments
Test
(1.05 KB, patch)
2012-02-16 22:05 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(7.69 KB, patch)
2012-02-16 22:19 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(7.50 KB, patch)
2012-02-16 22:22 PST
,
Shinya Kawanaka
haraken
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1
2012-02-16 22:05:36 PST
Created
attachment 127521
[details]
Test
Kentaro Hara
Comment 2
2012-02-16 22:12:43 PST
Comment on
attachment 127521
[details]
Test View in context:
https://bugs.webkit.org/attachment.cgi?id=127521&action=review
> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1687 > + # A DOMObject that is an ActiveDOMObject and also a DOMNode should be treated as an DOMNode here. > + # setJSWrapperForDOMNode() will look if node is active and choose correct map to add node to.
The change looks OK, but the comment is confusing to me. Maybe you can just remove the comment. Why "should" an ActiveDOMObject-and-DOMNode object be treated as DOMNode? In my understanding, - There is no ActiveDOMObject-and-DOMNode object for now. - If an ActiveDOMObject-and-DOMNode appeared in the future, we need another logic, like "setJSWrapperForActiveDOMNodeObject()".
Shinya Kawanaka
Comment 3
2012-02-16 22:19:45 PST
Created
attachment 127524
[details]
Patch
Shinya Kawanaka
Comment 4
2012-02-16 22:22:11 PST
(In reply to
comment #2
)
> (From update of
attachment 127521
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=127521&action=review
> > > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1687 > > + # A DOMObject that is an ActiveDOMObject and also a DOMNode should be treated as an DOMNode here. > > + # setJSWrapperForDOMNode() will look if node is active and choose correct map to add node to. > > The change looks OK, but the comment is confusing to me. Maybe you can just remove the comment. > > Why "should" an ActiveDOMObject-and-DOMNode object be treated as DOMNode? In my understanding, > - There is no ActiveDOMObject-and-DOMNode object for now. > - If an ActiveDOMObject-and-DOMNode appeared in the future, we need another logic, like "setJSWrapperForActiveDOMNodeObject()".
Thanks. I've removed the comment.
Shinya Kawanaka
Comment 5
2012-02-16 22:22:14 PST
Created
attachment 127525
[details]
Patch
Kentaro Hara
Comment 6
2012-02-16 22:25:15 PST
Comment on
attachment 127525
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=127525&action=review
> LayoutTests/ChangeLog:17 > + * platform/wincairo/Skipped:
platform/wk2/Skipped might be needed...? (I am not sure.)
> LayoutTests/fast/dom/shadow/shadow-root-new.html:9 > +<div id="container"></div>
Nit: This line is not necessary.
Shinya Kawanaka
Comment 7
2012-02-16 22:31:18 PST
(In reply to
comment #6
)
> (From update of
attachment 127525
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=127525&action=review
> > > LayoutTests/ChangeLog:17 > > + * platform/wincairo/Skipped: > > platform/wk2/Skipped might be needed...? (I am not sure.)
No one has added Skipped line for SHADOW_DOM there, but let's add this time for defensive style :-)
> > > LayoutTests/fast/dom/shadow/shadow-root-new.html:9 > > +<div id="container"></div> > > Nit: This line is not necessary.
Thanks. After fixing them, I'll land this.
Shinya Kawanaka
Comment 8
2012-02-16 22:42:41 PST
Committed
r108035
: <
http://trac.webkit.org/changeset/108035
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug