Bug 102606

Summary: Simulated mouse events should return an accurate offset.
Product: WebKit Reporter: Jon Lee <jonlee>
Component: UI EventsAssignee: Jon Lee <jonlee>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, beidson, mitz, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.8   
Bug Depends on:    
Bug Blocks: 98318    
Attachments:
Description Flags
Patch beidson: review+

Description Jon Lee 2012-11-17 22:24:20 PST
The offset returned by simulated mouse events is the same as the page event because we skip recalculation of the offset for simulated events.

The check for this bit has been in the code since the KDE days, I believe. I think this check should be removed because that offset is now calculated from scratch and cached, whereas back in those days the offset was assigned directly to the event instance, and therefore it was inappropriate to make additional adjustments to the value.

See changelist 14916: http://trac.webkit.org/changeset/14916
Comment 1 Jon Lee 2012-11-17 22:49:42 PST
<rdar://problem/12725627>
Comment 2 Jon Lee 2012-11-17 22:58:00 PST
Created attachment 174843 [details]
Patch
Comment 3 Brady Eidson 2012-11-17 23:08:01 PST
Comment on attachment 174843 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        Remove an old check to avoid adjusting the offset location, because we now recalculate the
> +        offset from scratch and cache it.

It took me awhile to realize what you meant here.  I thought you were implying that this change also made us cache the offset.

Maybe clarify something like:
"Originally we did the isSimulated check.  Then sometime later, we started caching the offset.  I realized that caching the offset made the isSimulated check irrelevant, so lets remove it"
Comment 4 Jon Lee 2012-11-17 23:50:00 PST
(In reply to comment #3)
> (From update of attachment 174843 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174843&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Remove an old check to avoid adjusting the offset location, because we now recalculate the
> > +        offset from scratch and cache it.
> 
> It took me awhile to realize what you meant here.  I thought you were implying that this change also made us cache the offset.
> 
> Maybe clarify something like:
> "Originally we did the isSimulated check.  Then sometime later, we started caching the offset.  I realized that caching the offset made the isSimulated check irrelevant, so lets remove it"

Yes, this needs better explanation. i will clarify in the submitted changelog.
Comment 5 Jon Lee 2012-11-18 00:04:36 PST
Committed r135065: <http://trac.webkit.org/changeset/135065>