Bug 102606 - Simulated mouse events should return an accurate offset.
Summary: Simulated mouse events should return an accurate offset.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.8
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks: 98318
  Show dependency treegraph
 
Reported: 2012-11-17 22:24 PST by Jon Lee
Modified: 2012-11-18 00:04 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.43 KB, patch)
2012-11-17 22:58 PST, Jon Lee
beidson: review+
Details | Formatted Diff | Diff

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