Some questions:4- Then WebViewImpl gets the adjust point based his test result target, and does some more upwards traversals, checking which ancestor has a hand cursor style applied.3- the original hit test result then is manipulated to become point based whose target is the "best node" from tap highlight.Hi Rick/blink-dev.2- touch adjust does some manipulations (which btw look to me a bit tricky, maybe over expensive/complicated given the amount of traversals that happen), and then a best target is found.
I was writing this on IRC, but given that it became a bit large, I decided to email you and blink-dev instead.
From my understanding, tap highlight today works as following:
1- a rect-based hit test is performed, and then result is passed to touch adjust.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/TouchAdjustment.cpp&cl=GROK&l=247&gsn=bestClickableNodeForHitTestResult&rcl=1423457752
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/EventHandler.cpp&sq=package:chromium&type=cs&l=2764&rcl=1423457752
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp&q=WebViewImpl::bestTapNode&sq=package:chromium&type=cs&l=1277
5- then it walks its ancestors upwards again to find the biggest ancestor biggest that has a cursor hand style applied.
6- this is our best node, if any.- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"- I'd say perf-wise, there could do some simplifications on (2). Is there any ongoing work on that area? I mean I've seen the effort to decrease the amount of hit tests performed on touch handling, but I am specifically talking about "touch adjustment" optimizations.--Antonio Gomes
Hi Rick/blink-dev.2- touch adjust does some manipulations (which btw look to me a bit tricky, maybe over expensive/complicated given the amount of traversals that happen), and then a best target is found.
I was writing this on IRC, but given that it became a bit large, I decided to email you and blink-dev instead.
From my understanding, tap highlight today works as following:
1- a rect-based hit test is performed, and then result is passed to touch adjust.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/TouchAdjustment.cpp&cl=GROK&l=247&gsn=bestClickableNodeForHitTestResult&rcl=1423457752
3- the original hit test result then is manipulated to become point based whose target is the "best node" from tap highlight.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/EventHandler.cpp&sq=package:chromium&type=cs&l=2764&rcl=1423457752
4- Then WebViewImpl gets the adjust point based his test result target, and does some more upwards traversals, checking which ancestor has a hand cursor style applied.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp&q=WebViewImpl::bestTapNode&sq=package:chromium&type=cs&l=1277
5- then it walks its ancestors upwards again to find the biggest ancestor biggest that has a cursor hand style applied.Some questions:
6- this is our best node, if any.- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"
- I'd say perf-wise, there could do some simplifications on (2). Is there any ongoing work on that area? I mean I've seen the effort to decrease the amount of hit tests performed on touch handling, but I am specifically talking about "touch adjustment" optimizations.
--Antonio Gomes
On Tue, Feb 10, 2015 at 5:40 AM, Antonio Gomes <toni...@gmail.com> wrote:Hi Rick/blink-dev.2- touch adjust does some manipulations (which btw look to me a bit tricky, maybe over expensive/complicated given the amount of traversals that happen), and then a best target is found.
I was writing this on IRC, but given that it became a bit large, I decided to email you and blink-dev instead.
From my understanding, tap highlight today works as following:
1- a rect-based hit test is performed, and then result is passed to touch adjust.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/TouchAdjustment.cpp&cl=GROK&l=247&gsn=bestClickableNodeForHitTestResult&rcl=1423457752Yes this part is pretty tricky. It's evolved over time to avoid around issues with touch adjustment in practice. Note that this stage, our primary use case is where to send the 'click' event (I wouldn't want to imply that this work is done for touch highlighting - highlighting just piggy-backs on the targeting logic since we want it to be consistent with the node that will be activated).3- the original hit test result then is manipulated to become point based whose target is the "best node" from tap highlight.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/EventHandler.cpp&sq=package:chromium&type=cs&l=2764&rcl=1423457752As the comment says, I consider this step to be due to bugs in rect-based hit-testing. Ideally we'd be able to use the rect-based result as is but there's a number of special cases we'd have to work through to make that usable without breaking website compat. I tried to simplify this and had a long tail of site compat issues that ultimately forced me to add this point-based test back in.4- Then WebViewImpl gets the adjust point based his test result target, and does some more upwards traversals, checking which ancestor has a hand cursor style applied.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp&q=WebViewImpl::bestTapNode&sq=package:chromium&type=cs&l=1277Right, now we're talking explicitly about link highlighting. This is a heuristic to try to make the highlight match the developers intention about what the conceptually activatable node is.5- then it walks its ancestors upwards again to find the biggest ancestor biggest that has a cursor hand style applied.Some questions:
6- this is our best node, if any.- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"Agreed. Perhaps 'cursor: pointer' should be in our default stylesheet for buttons (it is for links, why not buttons?).- I'd say perf-wise, there could do some simplifications on (2). Is there any ongoing work on that area? I mean I've seen the effort to decrease the amount of hit tests performed on touch handling, but I am specifically talking about "touch adjustment" optimizations.There isn't really active work here, no. Improving touch adjustment (simplicity, performance, effectiveness) has been on my team's back-burner for awhile but it's never really become important enough to be prioritized about the other things we're working on. I'd love to have someone working on this though.
- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"
Agreed. Perhaps 'cursor: pointer' should be in our default stylesheet for buttons (it is for links, why not buttons?).
On Tue, Feb 10, 2015 at 5:40 AM, Antonio Gomes <toni...@gmail.com> wrote:
Hi Rick/blink-dev.
(...)
1- a rect-based hit test is performed, and then result is passed to touch adjust.
2- touch adjust does some manipulations (which btw look to me a bit tricky, maybe over expensive/complicated given the amount of traversals that happen), and then a best target is found.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/TouchAdjustment.cpp&cl=GROK&l=247&gsn=bestClickableNodeForHitTestResult&rcl=1423457752
Yes this part is pretty tricky. It's evolved over time to avoid around issues with touch adjustment in practice. Note that this stage, our primary use case is where to send the 'click' event (I wouldn't want to imply that this work is done for touch highlighting - highlighting just piggy-backs on the targeting logic since we want it to be consistent with the node that will be activated).
3- the original hit test result then is manipulated to become point based whose target is the "best node" from tap highlight.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/EventHandler.cpp&sq=package:chromium&type=cs&l=2764&rcl=1423457752As the comment says, I consider this step to be due to bugs in rect-based hit-testing. Ideally we'd be able to use the rect-based result as is but there's a number of special cases we'd have to work through to make that usable without breaking website compat. I tried to simplify this and had a long tail of site compat issues that ultimately forced me to add this point-based test back in.
4- Then WebViewImpl gets the adjust point based his test result target, and does some more upwards traversals, checking which ancestor has a hand cursor style applied.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp&q=WebViewImpl::bestTapNode&sq=package:chromium&type=cs&l=1277Right, now we're talking explicitly about link highlighting. This is a heuristic to try to make the highlight match the developers intention about what the conceptually activatable node is.5- then it walks its ancestors upwards again to find the biggest ancestor biggest that has a cursor hand style applied.Some questions:
6- this is our best node, if any.- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"
Agreed. Perhaps 'cursor: pointer' should be in our default stylesheet for buttons (it is for links, why not buttons?).- I'd say perf-wise, there could do some simplifications on (2). Is there any ongoing work on that area? I mean I've seen the effort to decrease the amount of hit tests performed on touch handling, but I am specifically talking about "touch adjustment" optimizations.There isn't really active work here, no. Improving touch adjustment (simplicity, performance, effectiveness) has been on my team's back-burner for awhile but it's never really become important enough to be prioritized about the other things we're working on. I'd love to have someone working on this though.
However, we have been experimenting with the idea of disabling touch adjustment entirely on mobile-optimized sites (i.e. treating it only as a legacy compat thing for sites designed without touch in mind). If we could do this without degradation of user experience it would be the best possible result in terms of simplification and performance I think. WDYT?
Current blocker is our default checkboxes are too hard to activate without adjustment, but that should be fixable.
Hi Rick. Comments below. Cheers,On Tue, Feb 10, 2015 at 8:35 PM, Rick Byers <rby...@chromium.org> wrote:On Tue, Feb 10, 2015 at 5:40 AM, Antonio Gomes <toni...@gmail.com> wrote:Hi Rick/blink-dev.2- touch adjust does some manipulations (which btw look to me a bit tricky, maybe over expensive/complicated given the amount of traversals that happen), and then a best target is found.
(...)
1- a rect-based hit test is performed, and then result is passed to touch adjust.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/TouchAdjustment.cpp&cl=GROK&l=247&gsn=bestClickableNodeForHitTestResult&rcl=1423457752Yes this part is pretty tricky. It's evolved over time to avoid around issues with touch adjustment in practice. Note that this stage, our primary use case is where to send the 'click' event (I wouldn't want to imply that this work is done for touch highlighting - highlighting just piggy-backs on the targeting logic since we want it to be consistent with the node that will be activated).I see. A simplification of this code would also have to consider double tap zoom and context menu targets calculations, etcBack of the old WebKit/BackBerry times (which was removed from webkit.org upstream last year), the solution used to consist in:
- rect hit test returns all nodes interesting with the given area.
- a routine traversed that list, skipping non-clickable elements, and checking which clickable element had the biggest intersection area with the given touch area.- target point was adjusted to the middle of the intersected area
- (explicitly/implicitly z-ordering layers were taken into account).Also at some point there were some discussion that the visual feedback of "active state" was something that was wanted to avoid to users when tapping.I explain: when something clickable was tapped, the highlight was the only visual feedback users would get. The "active state" where clicked links used to change its font color to "red" did not happen.It was visually clean to users.
3- the original hit test result then is manipulated to become point based whose target is the "best node" from tap highlight.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/page/EventHandler.cpp&sq=package:chromium&type=cs&l=2764&rcl=1423457752As the comment says, I consider this step to be due to bugs in rect-based hit-testing. Ideally we'd be able to use the rect-based result as is but there's a number of special cases we'd have to work through to make that usable without breaking website compat. I tried to simplify this and had a long tail of site compat issues that ultimately forced me to add this point-based test back in.That (the root bug) is something good to fix at the hit test level, as you said. I will try to look at it.
4- Then WebViewImpl gets the adjust point based his test result target, and does some more upwards traversals, checking which ancestor has a hand cursor style applied.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp&q=WebViewImpl::bestTapNode&sq=package:chromium&type=cs&l=1277Right, now we're talking explicitly about link highlighting. This is a heuristic to try to make the highlight match the developers intention about what the conceptually activatable node is.5- then it walks its ancestors upwards again to find the biggest ancestor biggest that has a cursor hand style applied.Some questions:
6- this is our best node, if any.- steps (4) and (5) fix real world bugs like https://code.google.com/p/chromium/issues/detail?id=322323, but simple cases where I was expecting tap highlight to show up are getting off the road, e.g.:<input type="button" onclick=""> Click me!!!</button>
Adding style=cursor:pointer "fixes"I shared Elliot's dislike on this solution, though it would work I think.
What bothers me with the current Touch adjustment algorithm is that text nodes are not excluded right way from the valid clickage targets list.If I have have<input type=button>click me!</button>#text: "click me!" is what is returned as the clickable target from TouchAdjustment. This is conceptually wrong, I would say.
But as mentioned, even if this is fixed, and <input> was returned, it would continue not being highlighted because it has no cursor:hand style by default.Agreed. Perhaps 'cursor: pointer' should be in our default stylesheet for buttons (it is for links, why not buttons?).- I'd say perf-wise, there could do some simplifications on (2). Is there any ongoing work on that area? I mean I've seen the effort to decrease the amount of hit tests performed on touch handling, but I am specifically talking about "touch adjustment" optimizations.There isn't really active work here, no. Improving touch adjustment (simplicity, performance, effectiveness) has been on my team's back-burner for awhile but it's never really become important enough to be prioritized about the other things we're working on. I'd love to have someone working on this though.Does blink have good layout test coverage for the compat cases you mentioned above? If so, I would like to experiment with an alternative algorithm/approach.
However, we have been experimenting with the idea of disabling touch adjustment entirely on mobile-optimized sites (i.e. treating it only as a legacy compat thing for sites designed without touch in mind). If we could do this without degradation of user experience it would be the best possible result in terms of simplification and performance I think. WDYT?Sounds like a good thing.
Current blocker is our default checkboxes are too hard to activate without adjustment, but that should be fixable.Agreed too.
----Antonio Gomes