Bug 80501 - Remove Node::isContentElement and Node::isShadowElement
Summary: Remove Node::isContentElement and Node::isShadowElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shinya Kawanaka
URL:
Keywords:
Depends on:
Blocks: 78596
  Show dependency treegraph
 
Reported: 2012-03-07 00:59 PST by Shinya Kawanaka
Modified: 2012-03-08 00:22 PST (History)
9 users (show)

See Also:


Attachments
Patch (11.75 KB, patch)
2012-03-07 03:06 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Build test (13.08 KB, patch)
2012-03-07 18:55 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (8.89 KB, patch)
2012-03-07 21:47 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (10.05 KB, patch)
2012-03-07 22:50 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (9.36 KB, patch)
2012-03-07 22:56 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (7.86 KB, patch)
2012-03-07 23:00 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (7.75 KB, patch)
2012-03-07 23:48 PST, Shinya Kawanaka
morrita: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinya Kawanaka 2012-03-07 00:59:24 PST
We can remove these methods to use isInsertionPoint.
Comment 1 Shinya Kawanaka 2012-03-07 03:06:59 PST
Created attachment 130585 [details]
Patch
Comment 2 Build Bot 2012-03-07 03:29:48 PST
Comment on attachment 130585 [details]
Patch

Attachment 130585 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11833954
Comment 3 Shinya Kawanaka 2012-03-07 04:21:22 PST
Ah, symbols are necessary? I'll add them after EWS building windows port.
Comment 4 WebKit Review Bot 2012-03-07 05:04:59 PST
Comment on attachment 130585 [details]
Patch

Attachment 130585 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11850020

New failing tests:
fast/dom/shadow/content-element-user-agent-shadow.html
Comment 5 Gustavo Noronha (kov) 2012-03-07 05:15:46 PST
Comment on attachment 130585 [details]
Patch

Attachment 130585 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11839973
Comment 6 Dimitri Glazkov (Google) 2012-03-07 09:04:21 PST
Comment on attachment 130585 [details]
Patch

Looks great! please fix the build failures before landing.
Comment 7 Shinya Kawanaka 2012-03-07 18:55:01 PST
Created attachment 130747 [details]
Build test
Comment 8 Shinya Kawanaka 2012-03-07 21:25:13 PST
I found this approach is very difficult now.
We have RuntimeEnabledFeatures::shadowDOMEnabled() now, and the tagName of <content> is changed by the flag...
Comment 9 Shinya Kawanaka 2012-03-07 21:36:45 PST
(In reply to comment #8)
> I found this approach is very difficult now.
> We have RuntimeEnabledFeatures::shadowDOMEnabled() now, and the tagName of <content> is changed by the flag...

I found I was wrong...
The content elements in <details> and <summary> should always have HTMLNames::webkitShadowContentTag.
Comment 10 Shinya Kawanaka 2012-03-07 21:47:49 PST
Created attachment 130766 [details]
Patch
Comment 11 Build Bot 2012-03-07 22:09:26 PST
Comment on attachment 130766 [details]
Patch

Attachment 130766 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11863164
Comment 12 Shinya Kawanaka 2012-03-07 22:50:51 PST
Created attachment 130773 [details]
Patch
Comment 13 Shinya Kawanaka 2012-03-07 22:56:24 PST
Created attachment 130776 [details]
Patch
Comment 14 Shinya Kawanaka 2012-03-07 23:00:00 PST
Created attachment 130777 [details]
Patch
Comment 15 Hajime Morrita 2012-03-07 23:43:34 PST
Comment on attachment 130777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130777&action=review

> Source/WebCore/html/shadow/InsertionPoint.h:54
> +    virtual bool isInsertionPoint() const { return true; }

OVERRIDE here.

> Source/WebCore/html/shadow/InsertionPoint.h:77
> +    // https://bugs.webkit.org/show_bug.cgi?id=78596

It looks we don't need something special for <shadow>.
Comment 16 Shinya Kawanaka 2012-03-07 23:48:57 PST
Created attachment 130780 [details]
Patch
Comment 17 Shinya Kawanaka 2012-03-07 23:49:56 PST
(In reply to comment #16)
> Created an attachment (id=130780) [details]
> Patch

(In reply to comment #15)
> (From update of attachment 130777 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130777&action=review
> 
> > Source/WebCore/html/shadow/InsertionPoint.h:54
> > +    virtual bool isInsertionPoint() const { return true; }
> 
> OVERRIDE here.

Done.

> 
> > Source/WebCore/html/shadow/InsertionPoint.h:77
> > +    // https://bugs.webkit.org/show_bug.cgi?id=78596
> 
> It looks we don't need something special for <shadow>.

Done.
Comment 18 Hajime Morrita 2012-03-08 00:00:53 PST
Comment on attachment 130780 [details]
Patch

Looks OK.
Comment 19 Shinya Kawanaka 2012-03-08 00:22:32 PST
Committed r110151: <http://trac.webkit.org/changeset/110151>