Summary: | Factor EventContext and introduces MouseOrFocusEventContext. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hayato Ito <hayato> | ||||||
Component: | UI Events | Assignee: | Hayato Ito <hayato> | ||||||
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: | 109156 | ||||||||
Attachments: |
|
Description
Hayato Ito
2013-02-08 02:59:40 PST
Created attachment 187277 [details]
Factor EventContext
Attachment 187277 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/EventContext.cpp', u'Source/WebCore/dom/EventContext.h', u'Source/WebCore/dom/EventDispatcher.cpp', u'Source/WebCore/dom/EventDispatcher.h', u'Source/WebCore/inspector/InspectorInstrumentation.cpp', u'Source/WebCore/inspector/InspectorInstrumentation.h']" exit_code: 1
Source/WebCore/dom/EventContext.h:65: The parameter name "target" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 1 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 187277 [details] Factor EventContext View in context: https://bugs.webkit.org/attachment.cgi?id=187277&action=review > Source/WebCore/dom/EventContext.h:61 > +typedef Vector<OwnPtr<EventContext>, 32> EventContextAncestors; I am having trouble with this name. It seems more like an EventContextList. The word "ancestors" is completely out of place here, though I personally understand where it came from. Another idea: we could use the DOM core terminology of "event path" (http://dom.spec.whatwg.org/#dispatching-events) and instead of EventContextAncestors and ensureEventContextAncestors (neither make good sense), we just say "EventPath" and "ensureEventPath"? >> Source/WebCore/dom/EventContext.h:65 >> + MouseOrFocusEventContext(PassRefPtr<Node>, PassRefPtr<EventTarget> currentTarget, PassRefPtr<EventTarget> target); > > The parameter name "target" adds no information, so it should be removed. [readability/parameter_name] [5] Style elf is mistaken! :) Thank you for the review. Let me land this after addressing the comment. (In reply to comment #3) > (From update of attachment 187277 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187277&action=review > > > Source/WebCore/dom/EventContext.h:61 > > +typedef Vector<OwnPtr<EventContext>, 32> EventContextAncestors; > > I am having trouble with this name. It seems more like an EventContextList. The word "ancestors" is completely out of place here, though I personally understand where it came from. > > Another idea: we could use the DOM core terminology of "event path" (http://dom.spec.whatwg.org/#dispatching-events) and instead of EventContextAncestors and ensureEventContextAncestors (neither make good sense), we just say "EventPath" and "ensureEventPath"? I like it. Let me use that. We should use terminology from the spec unless there is a strong reason. We don't have a strong reason. :) > > >> Source/WebCore/dom/EventContext.h:65 > >> + MouseOrFocusEventContext(PassRefPtr<Node>, PassRefPtr<EventTarget> currentTarget, PassRefPtr<EventTarget> target); > > > > The parameter name "target" adds no information, so it should be removed. [readability/parameter_name] [5] > > Style elf is mistaken! :) Yeah :) Created attachment 187761 [details]
Patch for landing
Comment on attachment 187761 [details] Patch for landing Clearing flags on attachment: 187761 Committed r142575: <http://trac.webkit.org/changeset/142575> All reviewed patches have been landed. Closing bug. |