Bug 76414 - Clean EventContext class and move phase-tweaking logic to EventDispatcher class.
Summary: Clean EventContext class and move phase-tweaking logic to EventDispatcher class.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 76513
  Show dependency treegraph
 
Reported: 2012-01-16 18:26 PST by Hayato Ito
Modified: 2012-01-17 21:50 PST (History)
2 users (show)

See Also:


Attachments
clean EventContext (4.07 KB, patch)
2012-01-16 20:41 PST, Hayato Ito
no flags Details | Formatted Diff | Diff
update (4.08 KB, patch)
2012-01-17 20:25 PST, Hayato Ito
no flags Details | Formatted Diff | Diff
update (4.08 KB, patch)
2012-01-17 20:46 PST, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2012-01-16 18:26:38 PST
Clean EventContext class.
See https://bugs.webkit.org/show_bug.cgi?id=76217#c10 for more info.

This should be only refactoring, no functional changes.
Comment 1 Hayato Ito 2012-01-16 20:41:26 PST
Created attachment 122707 [details]
clean EventContext
Comment 2 Dimitri Glazkov (Google) 2012-01-17 09:47:19 PST
Comment on attachment 122707 [details]
clean EventContext

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

> Source/WebCore/dom/EventDispatcher.cpp:311
> +        const EventContext& eventContext = m_ancestors[i-1];

I think you can do even better here: instead of exposing currentTarget, add a method that checks for equality of target and currentTarget (adjustedTargetSameAsTarget? atTarget?). This way, you aren't exposing unnecessary information from EventContext.
Comment 3 Hayato Ito 2012-01-17 20:25:44 PST
Created attachment 122865 [details]
update
Comment 4 Hayato Ito 2012-01-17 20:27:13 PST
Thank you for the review. I've updated the patch.

(In reply to comment #2)
> (From update of attachment 122707 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122707&action=review
> 
> > Source/WebCore/dom/EventDispatcher.cpp:311
> > +        const EventContext& eventContext = m_ancestors[i-1];
> 
> I think you can do even better here: instead of exposing currentTarget, add a method that checks for equality of target and currentTarget (adjustedTargetSameAsTarget? atTarget?). This way, you aren't exposing unnecessary information from EventContext.

I named it EventContext::currentTargetSameAsTarget.
Comment 5 Hayato Ito 2012-01-17 20:46:47 PST
Created attachment 122867 [details]
update
Comment 6 Hayato Ito 2012-01-17 20:49:54 PST
Ops. I updated the patch without the previous patch got r+.
The new patch fixes one style issue. No behavior change. Could you review it again?
Comment 7 WebKit Review Bot 2012-01-17 21:50:11 PST
Comment on attachment 122867 [details]
update

Clearing flags on attachment: 122867

Committed r105241: <http://trac.webkit.org/changeset/105241>
Comment 8 WebKit Review Bot 2012-01-17 21:50:28 PST
All reviewed patches have been landed.  Closing bug.