Bug 85101

Summary:

[chromium] Compute the best target node on a GestureTap event

Product:

WebKit

Reporter:

Terry Anderson <tdanderson>

Component:

Platform

Assignee:

Terry Anderson <tdanderson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

allan.jensen, dglazkov, rjkroege, trchen, webkit.review.bot, wjmaclean

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

85289, 85296, 86083, 86218, 86260, 86397

Bug Blocks:

83947, 84487

Attachments:

Description	Flags
Patch	none
Patch	webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Diffs for two failing touch adjustment layout tests in chromium	none
Archive of layout-test-results from ec2-cr-linux-02	none
Patch	none
Diff for failing test touchadjustment/touch-inlines.html	none
Patch	none
Patch	none

Terry Anderson

Reported 2012-04-27 14:47:34 PDT

Use the new function WebViewImpl::computeBestTouchTarget determine the most likely target Node given a rectangular area as input. Still to do: Refactor to avoid multiple hit tests.

Attachments
Patch (9.32 KB, patch) 2012-04-27 14:53 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (8.97 KB, patch) 2012-04-30 15:54 PDT, Terry Anderson	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.26 MB, application/zip) 2012-04-30 17:20 PDT, WebKit Review Bot	no flags	Details
Patch (5.01 KB, patch) 2012-05-01 11:08 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (5.59 KB, patch) 2012-05-01 13:03 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (15.24 KB, patch) 2012-05-03 17:21 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (8.13 KB, patch) 2012-05-07 16:26 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (6.50 KB, patch) 2012-05-10 08:00 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (4.93 KB, patch) 2012-05-10 14:15 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Diffs for two failing touch adjustment layout tests in chromium (1.73 KB, text/plain) 2012-05-10 14:19 PDT, Terry Anderson	no flags	Details
Archive of layout-test-results from ec2-cr-linux-02 (860.96 KB, application/zip) 2012-05-10 21:56 PDT, WebKit Review Bot	no flags	Details
Patch (5.42 KB, patch) 2012-05-11 10:07 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Diff for failing test touchadjustment/touch-inlines.html (864 bytes, text/plain) 2012-05-14 08:00 PDT, Terry Anderson	no flags	Details
Patch (5.58 KB, patch) 2012-05-14 10:25 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Patch (6.79 KB, patch) 2012-05-15 10:16 PDT, Terry Anderson	no flags	Details Formatted Diff Diff
Show Obsolete (12) View All Add attachment proposed patch, testcase, etc.

Terry Anderson

Comment 1 2012-04-27 14:53:20 PDT

Created attachment 139282 [details] Patch

WebKit Review Bot

Comment 2 2012-04-27 14:58:58 PDT

Attachment 139282 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Terry Anderson

Comment 3 2012-04-30 15:54:00 PDT

Created attachment 139535 [details] Patch

WebKit Review Bot

Comment 4 2012-04-30 15:57:31 PDT

Attachment 139535 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 5 2012-04-30 17:20:51 PDT

Comment on attachment 139535 [details] Patch Attachment 139535 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12585441 New failing tests: fast/events/touch/gesture/gesture-click.html fast/events/touch/page-scaled-touch-gesture-click.html

WebKit Review Bot

Comment 6 2012-04-30 17:20:56 PDT

Created attachment 139550 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Terry Anderson

Comment 7 2012-05-01 11:08:36 PDT

Created attachment 139653 [details] Patch

Terry Anderson

Comment 8 2012-05-01 11:10:15 PDT

(In reply to comment #7) > Created an attachment (id=139653) [details] > Patch Depends on https://bugs.webkit.org/show_bug.cgi?id=85296 landing first, since this patch calls WebCore::EventHandler::handleGestureTap with an extra parameter.

WebKit Review Bot

Comment 9 2012-05-01 11:23:39 PDT

Comment on attachment 139653 [details] Patch Attachment 139653 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12588623

Robert Kroeger

Comment 10 2012-05-01 11:34:15 PDT

Comment on attachment 139653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139653&action=review > Source/WebKit/chromium/ChangeLog:3 > + [chromium] Compute the best target node on a GestureTap event My limited intuition of the way-of-WebKit is that this needs to be a bit more detailed. I want more detail and I participated in the design discussions so a reviewer probably needs more context. > Source/WebKit/chromium/src/WebViewImpl.cpp:647 > + // rectangle-based hit detection to find the best touch target afaik, these kinds of comments are usually not found in WebKit. > Source/WebKit/chromium/src/WebViewImpl.cpp:665 > + m_bestTouchTarget = 0; I think m_bestTouchTarget.clear() would be better? > Source/WebKit/chromium/src/WebViewImpl.cpp:958 > + return node; is this right usage?

Terry Anderson

Comment 11 2012-05-01 13:03:56 PDT

Created attachment 139661 [details] Patch

Terry Anderson

Comment 12 2012-05-01 13:06:53 PDT

Comment on attachment 139653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139653&action=review >> Source/WebKit/chromium/ChangeLog:3 >> + [chromium] Compute the best target node on a GestureTap event > > My limited intuition of the way-of-WebKit is that this needs to be a bit more detailed. I want more detail and I participated in the design discussions so a reviewer probably needs more context. I added a more thorough description in the ChangeLog. >> Source/WebKit/chromium/src/WebViewImpl.cpp:647 >> + // rectangle-based hit detection to find the best touch target > > afaik, these kinds of comments are usually not found in WebKit. Removed. >> Source/WebKit/chromium/src/WebViewImpl.cpp:665 >> + m_bestTouchTarget = 0; > > I think m_bestTouchTarget.clear() would be better? WebCore::Node does not have a clear() routine. >> Source/WebKit/chromium/src/WebViewImpl.cpp:958 >> + return node; > > is this right usage? I believe so, but I'm not 100% sure...

Terry Anderson

Comment 13 2012-05-01 13:09:00 PDT

(In reply to comment #11) > Created an attachment (id=139661) [details] > Patch Layout tests are in progress.

Darin Fisher (:fishd, Google)

Comment 14 2012-05-01 13:22:25 PDT

Comment on attachment 139661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139661&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:647 > + WebCore::IntPoint inputPoint(event.x, event.y); nit: no need for WebCore:: prefix > Source/WebKit/chromium/src/WebViewImpl.cpp:953 > + || node->hasEventListeners(eventNames().clickEvent) nit: please indent by four spaces instead of only one > Source/WebKit/chromium/src/WebViewImpl.h:792 > + RefPtr<WebCore::Node> m_bestTouchTarget; Why does this need to be a member variable?

Terry Anderson

Comment 15 2012-05-03 17:21:27 PDT

Created attachment 140131 [details] Patch

W. James MacLean

Comment 16 2012-05-04 06:45:07 PDT

Comment on attachment 140131 [details] Patch Looks reasonable (to me) in general, but I wonder if we should consider moving some or all of this out of WebViewImpl and into a utility class. This might allow it to be useful here and elsewhere, and I'm worried that WVI is becoming a bit of a catch-all class.

Robert Kroeger

Comment 17 2012-05-04 06:55:57 PDT

(In reply to comment #16) > (From update of attachment 140131 [details]) > Looks reasonable (to me) in general, but I wonder if we should consider moving some or all of this out of WebViewImpl and into a utility class. This might allow it to be useful here and elsewhere, and I'm worried that WVI is becoming a bit of a catch-all class. I've been worried about WVI bloat myself. This seems like an excellent suggestion.

Robert Kroeger

Comment 18 2012-05-04 07:00:26 PDT

Comment on attachment 140131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140131&action=review > Source/WebKit/chromium/ChangeLog:9 > + (WebKit::WebViewImpl::handleGestureEvent): Your ChangeLog doesn't seem formatted quite like the others. > Source/WebKit/chromium/ChangeLog:28 > + Note: I expect that this patch will cause progressions in two existing layout You mean regressions or failures? > Source/WebKit/chromium/src/WebViewImpl.h:599 > + // Returns the best touch target given a rectangle defined by a center point these can be inside enable(GESTURE_EVENTS) right? And I agree with wjmaclean@: a separate external utility class would probably be a good idea.

Terry Anderson

Comment 19 2012-05-07 16:26:46 PDT

Created attachment 140617 [details] Patch

WebKit Review Bot

Comment 20 2012-05-07 16:32:03 PDT

Attachment 140617 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.

Terry Anderson

Comment 21 2012-05-07 16:37:20 PDT

(In reply to comment #19) > Created an attachment (id=140617) [details] > Patch Change of plan: I would now like to use the TOUCH_ADJUSTMENT code in WebCore instead of re-writing this in WebViewImpl. After speaking with Allan today (carewolf on #webkit) I discovered that this code will work for chromium if the scroll offsets are taken into account. Allan: can you please comment on how my proposed changes will impact the qt platform? If needed I can always put some parts of this code behind a setting so that it doesn't get in your platform's way. I tried this out on a variety of pages and it seems to work fine, but I do get an "Aw, Snap" (crash) when dispatching a GestureTap event on the checkboxes here: http://www.w3schools.com/html/tryit.asp?filename=tryhtml_checkbox

Allan Sandfeld Jensen

Comment 22 2012-05-08 00:36:44 PDT

(In reply to comment #21) > (In reply to comment #19) > > Created an attachment (id=140617) [details] [details] > > Patch > > Change of plan: I would now like to use the TOUCH_ADJUSTMENT code in WebCore instead of re-writing this in WebViewImpl. After speaking with Allan today (carewolf on #webkit) I discovered that this code will work for chromium if the scroll offsets are taken into account. > > Allan: can you please comment on how my proposed changes will impact the qt platform? If needed I can always put some parts of this code behind a setting so that it doesn't get in your platform's way. > It shouldn't affect the Qt platform, as far as I can tell. The problem is we use a slightly different scrolling model. Please add regression-tests though. I have some touch-adjustment tests in LayoutTests/touchadjustment, but you should add tests with overflow and scroll-offset.

Allan Sandfeld Jensen

Comment 23 2012-05-08 00:43:27 PDT

Comment on attachment 140617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140617&action=review > Source/WebCore/page/EventHandler.cpp:2440 > + bestClickableNodeForTouchPoint(gestureEvent.position(), IntSize(gestureEvent.area().width() / 2, gestureEvent.area().height() / 2), adjustedPoint, targetNode, containingFrame); containingFrame can just as well be calculated in bestClickableNodeForTouchPoint as here, so I don't think there is any reason to give it as an argument. > Source/WebCore/page/EventHandler.cpp:2446 > + // convert back to window coordinates > + adjustedPoint.setX(adjustedPoint.x() - containingFrame->document()->view()->scrollX()); > + adjustedPoint.setY(adjustedPoint.y() - containingFrame->document()->view()->scrollY()); I think this conversion should be done in findBestClickableCandidate, preferable using contentsToWindow.

Robert Kroeger

Comment 24 2012-05-08 06:37:48 PDT

Comment on attachment 140617 [details] Patch Perhaps you could add some kind of window.internals based test for bestClickableNodeForTouchPoint? And/or the layout tests you were preparing?

Allan Sandfeld Jensen

Comment 25 2012-05-08 06:59:59 PDT

(In reply to comment #24) > (From update of attachment 140617 [details]) > Perhaps you could add some kind of window.internals based test for bestClickableNodeForTouchPoint? And/or the layout tests you were preparing? There is an internals API for it, just follow the examples in LayoutTests/touchadjustment.

Allan Sandfeld Jensen

Comment 26 2012-05-10 04:10:27 PDT

This depends on fixing the bug in touch-adjustment that scroll-offset was not properly handled. I am tracking that separately in bug #86083 now.

Terry Anderson

Comment 27 2012-05-10 08:00:05 PDT

Created attachment 141172 [details] Patch

Terry Anderson

Comment 28 2012-05-10 08:04:51 PDT

(In reply to comment #27) > Created an attachment (id=141172) [details] > Patch This patch is an improvement over my last one, as it appears to work for cases involving frames and scroll offsets. But in light of https://bugs.webkit.org/show_bug.cgi?id=86083 being filed (and about to land), I will have to re-revise.

Allan Sandfeld Jensen

Comment 29 2012-05-10 12:58:34 PDT

(In reply to comment #28) > (In reply to comment #27) > > Created an attachment (id=141172) [details] [details] > > Patch > > This patch is an improvement over my last one, as it appears to work for cases involving frames and scroll offsets. But in light of https://bugs.webkit.org/show_bug.cgi?id=86083 being filed (and about to land), I will have to re-revise. I apologize, but your arguments made me realize it was a general problem for both touch-adjustment functions, so I wanted a general solution. Hopefully this should work for you as well, otherwise feel free to improve it :)

Terry Anderson

Comment 30 2012-05-10 14:15:55 PDT

Created attachment 141251 [details] Patch

Terry Anderson

Comment 31 2012-05-10 14:19:27 PDT

Created attachment 141253 [details] Diffs for two failing touch adjustment layout tests in chromium In the chromium port, all of the layout tests in the touchadjustment/ directory pass except for two: touchadjustment/event-triggered-widgets.html touchadjustment/touch-inlines.html

WebKit Review Bot

Comment 32 2012-05-10 14:23:34 PDT

Attachment 141251 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Terry Anderson

Comment 33 2012-05-10 14:30:31 PDT

(In reply to comment #29) > (In reply to comment #28) > > (In reply to comment #27) > > > Created an attachment (id=141172) [details] [details] [details] > > > Patch > > > > This patch is an improvement over my last one, as it appears to work for cases involving frames and scroll offsets. But in light of https://bugs.webkit.org/show_bug.cgi?id=86083 being filed (and about to land), I will have to re-revise. > > I apologize, but your arguments made me realize it was a general problem for both touch-adjustment functions, so I wanted a general solution. Hopefully this should work for you as well, otherwise feel free to improve it :) I tried out the touch adjustment code in chromium after your patch (https://bugs.webkit.org/show_bug.cgi?id=86083) landed, and everything seemed to work great without making any other modifications to the code. However, two of the layout tests in touchadjustment/ fail (I have attached the diffs to this bug) and it is not immediately obvious to me why this is. Do you have any ideas about what might be causing this difference?

WebKit Review Bot

Comment 34 2012-05-10 21:56:18 PDT

Comment on attachment 141251 [details] Patch Attachment 141251 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12671230 New failing tests: touchadjustment/touch-inlines.html touchadjustment/event-triggered-widgets.html plugins/embed-attributes-style.html

WebKit Review Bot

Comment 35 2012-05-10 21:56:23 PDT

Created attachment 141325 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Allan Sandfeld Jensen

Comment 36 2012-05-11 07:18:31 PDT

(In reply to comment #33) > (In reply to comment #29) > > (In reply to comment #28) > > > (In reply to comment #27) > > > > Created an attachment (id=141172) [details] [details] [details] [details] > > > > Patch > > > > > > This patch is an improvement over my last one, as it appears to work for cases involving frames and scroll offsets. But in light of https://bugs.webkit.org/show_bug.cgi?id=86083 being filed (and about to land), I will have to re-revise. > > > > I apologize, but your arguments made me realize it was a general problem for both touch-adjustment functions, so I wanted a general solution. Hopefully this should work for you as well, otherwise feel free to improve it :) > > I tried out the touch adjustment code in chromium after your patch (https://bugs.webkit.org/show_bug.cgi?id=86083) landed, and everything seemed to work great without making any other modifications to the code. > > However, two of the layout tests in touchadjustment/ fail (I have attached the diffs to this bug) and it is not immediately obvious to me why this is. Do you have any ideas about what might be causing this difference? These two tests fail for me as well when using DumpRenderTree, but works fine when I use WebKitTestRunner. They also pass using DRT on our automatic testing bots, so I assume they are flaky somehow. I am going to investigate it, so feel free to skip them until I find out why they change depending on platform.

Terry Anderson

Comment 37 2012-05-11 10:07:31 PDT

Created attachment 141438 [details] Patch

Adam Barth

Comment 38 2012-05-11 10:10:39 PDT

Comment on attachment 141438 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141438&action=review > Source/WebCore/WebCore.gypi:3058 > + 'page/TouchAdjustment.cpp', > + 'page/TouchAdjustment.h', Did you mean to svn add these files?

Terry Anderson

Comment 39 2012-05-11 10:14:12 PDT

(In reply to comment #36) > (In reply to comment #33) > > (In reply to comment #29) > > > (In reply to comment #28) > > > > (In reply to comment #27) > > > > > Created an attachment (id=141172) [details] [details] [details] [details] [details] > > > > > Patch > > > > > > > > This patch is an improvement over my last one, as it appears to work for cases involving frames and scroll offsets. But in light of https://bugs.webkit.org/show_bug.cgi?id=86083 being filed (and about to land), I will have to re-revise. > > > > > > I apologize, but your arguments made me realize it was a general problem for both touch-adjustment functions, so I wanted a general solution. Hopefully this should work for you as well, otherwise feel free to improve it :) > > > > I tried out the touch adjustment code in chromium after your patch (https://bugs.webkit.org/show_bug.cgi?id=86083) landed, and everything seemed to work great without making any other modifications to the code. > > > > However, two of the layout tests in touchadjustment/ fail (I have attached the diffs to this bug) and it is not immediately obvious to me why this is. Do you have any ideas about what might be causing this difference? > > These two tests fail for me as well when using DumpRenderTree, but works fine when I use WebKitTestRunner. They also pass using DRT on our automatic testing bots, so I assume they are flaky somehow. I am going to investigate it, so feel free to skip them until I find out why they change depending on platform. I have skipped them for now. I just went through and performed all of these tests manually one by one and can confirm that they actually work on the chromium port. Should we address the strange behavior of DRT with these tests in a separate bug?

Adam Barth

Comment 40 2012-05-11 10:24:48 PDT

Comment on attachment 141438 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141438&action=review Generally, this patch looks fine. One small question below. >> Source/WebCore/WebCore.gypi:3058 >> + 'page/TouchAdjustment.h', > > Did you mean to svn add these files? /me discovers that they're already in WebKit. > Source/WebKit/chromium/src/WebInputEventConversion.cpp:151 > + m_area = IntSize(e.deltaX * 2, e.deltaY * 2); Where does the constant 2 come from? Why is this WebKit's job rather than the embedder's job to do this multiplication. (I don't know much about the division of responsibility here.)

Terry Anderson

Comment 41 2012-05-11 10:38:37 PDT

Comment on attachment 141438 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141438&action=review >> Source/WebKit/chromium/src/WebInputEventConversion.cpp:151 >> + m_area = IntSize(e.deltaX * 2, e.deltaY * 2); > > Where does the constant 2 come from? Why is this WebKit's job rather than the embedder's job to do this multiplication. (I don't know much about the division of responsibility here.) In chromium, I am using deltaX and deltaY to store the touch radii (i.e., the horizontal and vertical distances from the center of the touch region to the edge of the touch region) which in general can be given by the hardware as the radii of an elliptical touch region. What I'm doing here is using these two radii to define a rectangular touch region (of width deltaX*2 and height deltaY*2) to be used in the TouchAdjustment code.

Adam Barth

Comment 42 2012-05-11 10:48:12 PDT

Comment on attachment 141438 [details] Patch Thanks for the explanation. I still don't fully understand why we don't set m_area for each type of event, but we can always change that in the future if necessary.

WebKit Review Bot

Comment 43 2012-05-11 14:15:35 PDT

Comment on attachment 141438 [details] Patch Clearing flags on attachment: 141438 Committed r116802: <http://trac.webkit.org/changeset/116802>

WebKit Review Bot

Comment 44 2012-05-11 14:15:56 PDT

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 45 2012-05-11 15:32:14 PDT

Re-opened since this is blocked by 86260

Allan Sandfeld Jensen

Comment 46 2012-05-12 09:45:31 PDT

(In reply to comment #39) > I have skipped them for now. I just went through and performed all of these tests manually one by one and can confirm that they actually work on the chromium port. Should we address the strange behavior of DRT with these tests in a separate bug? Bug #86218, but it should be solved by now. Turned out to be pixel/metrics differences in font-rendering, which shifted the text enough to give different hit-test results.

Terry Anderson

Comment 47 2012-05-14 08:00:13 PDT

Created attachment 141730 [details] Diff for failing test touchadjustment/touch-inlines.html The test touchadjustment/touch-inlines.html still fails on chromium after patch https://bugs.webkit.org/show_bug.cgi?id=86218 was landed.

Terry Anderson

Comment 48 2012-05-14 10:25:05 PDT

Created attachment 141748 [details] Patch

Adam Barth

Comment 49 2012-05-14 11:22:29 PDT

Comment on attachment 141748 [details] Patch Ok.

WebKit Review Bot

Comment 50 2012-05-14 12:23:31 PDT

Comment on attachment 141748 [details] Patch Clearing flags on attachment: 141748 Committed r116983: <http://trac.webkit.org/changeset/116983>

WebKit Review Bot

Comment 51 2012-05-14 12:23:38 PDT

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 52 2012-05-14 13:42:55 PDT

Re-opened since this is blocked by 86397

Terry Anderson

Comment 53 2012-05-15 10:16:51 PDT

Created attachment 141992 [details] Patch

Terry Anderson

Comment 54 2012-05-15 10:17:32 PDT

(In reply to comment #53) > Created an attachment (id=141992) [details] > Patch This now compiles on chromium win: http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/29198

Adam Barth

Comment 55 2012-05-15 10:22:22 PDT

Comment on attachment 141992 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141992&action=review > Source/WebCore/page/TouchAdjustment.cpp:234 > - return INFINITY; > + return std::numeric_limits<float>::infinity(); Ah, that makes sense.

WebKit Review Bot

Comment 56 2012-05-15 12:58:12 PDT

Comment on attachment 141992 [details] Patch Clearing flags on attachment: 141992 Committed r117128: <http://trac.webkit.org/changeset/117128>

WebKit Review Bot

Comment 57 2012-05-15 12:58:20 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.