Bug 106614 - [Shadow DOM] Refactoring: InsertionPoint could simplify its subclass hooks
Summary: [Shadow DOM] Refactoring: InsertionPoint could simplify its subclass hooks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 106283
  Show dependency treegraph
 
Reported: 2013-01-10 16:16 PST by Hajime Morrita
Modified: 2013-01-10 20:01 PST (History)
3 users (show)

See Also:


Attachments
Patch (11.46 KB, patch)
2013-01-10 17:34 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (11.39 KB, patch)
2013-01-10 18:57 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (11.42 KB, patch)
2013-01-10 19:46 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.