Bug 106614

Summary: [Shadow DOM] Refactoring: InsertionPoint could simplify its subclass hooks
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: ojan.autocc, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106283    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Hajime Morrita 2013-01-10 16:16:00 PST
This is a preparation for Bug 106283.
Comment 1 Hajime Morrita 2013-01-10 17:34:15 PST
Created attachment 182229 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2013-01-10 18:19:44 PST
Comment on attachment 182229 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        - Introduces InsertionPoint::prematch() to give a chance each InsertionPoint to decide whether it

I don't like the word prematch. It doesn't convey the meaning here quite right. Can we treat this instead as a function to determine what kind of matching this insertion point uses:


MatchType {
AlwaysMatches
NeverMatches
HasToMatchSelector
}
Comment 3 Hajime Morrita 2013-01-10 18:57:57 PST
Created attachment 182242 [details]
Patch
Comment 4 Hajime Morrita 2013-01-10 18:59:09 PST
(In reply to comment #2)
> I don't like the word prematch. It doesn't convey the meaning here quite right. Can we treat this instead as a function to determine what kind of matching this insertion point uses:
Thanks for the insight! I didn't have good name here. Renamed.
Comment 5 Dimitri Glazkov (Google) 2013-01-10 19:30:37 PST
Comment on attachment 182242 [details]
Patch

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

> Source/WebCore/html/shadow/InsertionPoint.h:54
> +        MatchAccept,
> +        MatchReject,
> +        MatchSelect

I kind of like the longer names that explain what's happening (the ones that I suggested).
Comment 6 Hajime Morrita 2013-01-10 19:45:27 PST
(In reply to comment #5)
> I kind of like the longer names that explain what's happening (the ones that I suggested).
OK, will take that way.
Comment 7 Hajime Morrita 2013-01-10 19:46:26 PST
Created attachment 182245 [details]
Patch for landing
Comment 8 WebKit Review Bot 2013-01-10 20:01:37 PST
Comment on attachment 182245 [details]
Patch for landing

Clearing flags on attachment: 182245

Committed r139400: <http://trac.webkit.org/changeset/139400>
Comment 9 WebKit Review Bot 2013-01-10 20:01:42 PST
All reviewed patches have been landed.  Closing bug.