Bug 95996 - [chromium] Don't use WebGestureEvent.boundingBox for touch adjustment
Summary: [chromium] Don't use WebGestureEvent.boundingBox for touch adjustment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rick Byers
URL:
Keywords:
Depends on:
Blocks: 95496
  Show dependency tree / graph
 
Reported: 2012-09-06 10:42 PDT by Rick Byers
Modified: 2012-09-10 07:03 PDT (History)
6 users (show)

See Also:

Attachments
Patch (1.95 KB, patch)
2012-09-06 10:42 PDT, Rick Byers 		no flags Details | Formatted Diff | Diff
Patch (2.60 KB, patch)
2012-09-06 14:43 PDT, Rick Byers 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Byers 2012-09-06 10:42:07 PDT 
[chromium] Don't use WebGestureEvent.boundingBox for touch adjustment
Comment 1 Rick Byers 2012-09-06 10:42:58 PDT 
Created attachment 162538 [details]
Patch
Comment 2 Rick Byers 2012-09-06 10:44:50 PDT 
trchen@ It looks like you recently added another use of GestureEvent boundingBox, so I missed this one in transitioning all the usages to the new fields in bug 95573.  Does this simple change look OK to you?
Comment 3 Tien-Ren Chen 2012-09-06 11:21:26 PDT 
You may also want to comment out WebFrameTest.DisambiguationPopupTest for now.

I have one question regarding to the definition of width and height. So (event.x, event.y) is always the top left corner of the fat tap area, and the area extends to (event.x + event.width, event.y + event.height)?

From the chromium side change I thought (event.x, event.y) is the center and the area extends for (+-event.width, +-event.height).

Thanks for the help!
Comment 4 Rick Byers 2012-09-06 14:43:32 PDT 
Created attachment 162590 [details]
Patch
Comment 5 Rick Byers 2012-09-06 17:31:50 PDT 
(In reply to comment #3)
> You may also want to comment out WebFrameTest.DisambiguationPopupTest for now.

The fix looks simple enough, OK if I just make it?  I'm not sure how to run the test though - in my build webkit_unit_tests doesn't include WebFrameTests, but from the .gypi it doesn't appear to be Android specific or anything so not sure why.

> I have one question regarding to the definition of width and height. So (event.x, event.y) is always the top left corner of the fat tap area, and the area extends to (event.x + event.width, event.y + event.height)?
> 
> From the chromium side change I thought (event.x, event.y) is the center and the area extends for (+-event.width, +-event.height).

Yes, sorry - you're right.  (event.x, event.y) is the center, and width/height represent the total width, so bounding box extends from event.x-width/2 to event.x+width/2 etc.
Comment 6 Rick Byers 2012-09-08 07:32:50 PDT 
Ping - does this look OK to you know trchen@?
Comment 7 Adam Barth 2012-09-08 07:34:42 PDT 
Comment on attachment 162590 [details]
Patch

Looks reasonable to me.  Please check with trchen before landing.
Comment 8 Tien-Ren Chen 2012-09-10 02:52:46 PDT 
(In reply to comment #6)
> Ping - does this look OK to you know trchen@?

LGTM. Great thanks for the explaination!
Comment 9 Rick Byers 2012-09-10 05:48:04 PDT 
Great, thanks.  Can someone put this in the CQ for me please?  I want to get this gardened into chromium ASAP so I can start the process of removing these old fields before someone starts using them somewhere else ;-)
Comment 10 WebKit Review Bot 2012-09-10 07:03:01 PDT 
Comment on attachment 162590 [details]
Patch

Clearing flags on attachment: 162590

Committed r128058: <http://trac.webkit.org/changeset/128058>
Comment 11 WebKit Review Bot 2012-09-10 07:03:05 PDT 
All reviewed patches have been landed.  Closing bug.