Bug 92093 – There is no way to tell whether an element can be activated or not

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Allan Sandfeld Jensen]

To complement the hasEventListener we should have a corresponding hasDefaultEventHandler that can tell if a node implementation will react in any way to an event passed to it.

This is useful in touch-adjustment which can more reliably tell which nodes might respond to a synthesised click event and which will node.

[Allan Sandfeld Jensen]

Created attachment 154013 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 154013 [details]
Patch

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

> Source/WebCore/html/HTMLSummaryElement.cpp:160
> +bool HTMLSummaryElement::hasDefaultEventHandler(const AtomicString& eventType)

Why not call it hasDefaultEventHandlerFor ?

[WebKit Review Bot]

Comment on attachment 154013 [details]
Patch

Attachment 154013 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13331341

[Allan Sandfeld Jensen]

(In reply to comment #2)
> (From update of attachment 154013 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154013&action=review
> 
> > Source/WebCore/html/HTMLSummaryElement.cpp:160
> > +bool HTMLSummaryElement::hasDefaultEventHandler(const AtomicString& eventType)
> 
> Why not call it hasDefaultEventHandlerFor ?

It follows the argument style of hasEventListeners(), but I have no strong opinion on what we call it.

[Allan Sandfeld Jensen]

Created attachment 154022 [details]
Patch

[WebKit Review Bot]

Comment on attachment 154022 [details]
Patch

Attachment 154022 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13325438

[Kevin Ellis]

cr-linux error:

Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:204: error: 'const class WebCore::HTMLInputElement' has no member named 'isReadOnly'

Should this be isReadOnlyFormControl?

[Kevin Ellis]

Thanks for looking into this Allan.

Addresses one of two issues with bug 91894 (Search cancel button is hard to activate with a tap gesture even if touch adjustment is enabled).

[Allan Sandfeld Jensen]

(In reply to comment #7)
> cr-linux error:
> 
> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:204: error: 'const class WebCore::HTMLInputElement' has no member named 'isReadOnly'
> 
> Should this be isReadOnlyFormControl?

No it should be readOnly(). I will upload a new patch soon.

[Allan Sandfeld Jensen]

Created attachment 154037 [details]
Patch

Fix isReadOnly -> readOnly() and add hasDefaultEventHandler to shadow media controls

[Antonio Gomes]

Comment on attachment 154037 [details]
Patch

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

> Source/WebCore/ChangeLog:13
> +        Touch-adjustment is changed to using hasDefaultEventHandler which improves accuracy in 
> +        shadow-dom, and also means it will now prefer enabled form-elements over disabled ones.

is the difference testable?

[Kevin Ellis]

(In reply to comment #11)
> (From update of attachment 154037 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154037&action=review
> 
> > Source/WebCore/ChangeLog:13
> > +        Touch-adjustment is changed to using hasDefaultEventHandler which improves accuracy in 
> > +        shadow-dom, and also means it will now prefer enabled form-elements over disabled ones.
> 
> is the difference testable?

This patch partially fixes bug 91894 (search cancel button is hard to activate with a tap gesture even if touch adjustment is enabled).  Please feel free to borrow from that test case.  Without this fix, any touch that clips the input field next to the cancel button would cause a touch adjustment to the input field as the cancel button would be pruned from the candidate list.  In short, the fix is testable, though several cases in the test attached to 91894 will still fail because of the scoring algorithm.

[Alexey Proskuryakov]

I have very strong reservations about this feature.

This is a whole lot of parallel knowledge about default handlers, which can become a maintenance nightmare. Furthermore, it's hard to get interoperability here, because certain behaviors that are implemented via default handlers in one browser can be implemented otherwise in others (access keys come to mind).

[Ryosuke Niwa]

Comment on attachment 154037 [details]
Patch

This feature should definitely be discussed on webkit-dev first.

[Allan Sandfeld Jensen]

(In reply to comment #13)
> I have very strong reservations about this feature.
> 
> This is a whole lot of parallel knowledge about default handlers, which can become a maintenance nightmare. Furthermore, it's hard to get interoperability here, because certain behaviors that are implemented via default handlers in one browser can be implemented otherwise in others (access keys come to mind).

It is NOT intended as an exported API, I completely agree with you on that. This is only intended for internal use.

It is currently only used for touch-adjustment. The alternative to hasDefaultEventHandler could be an API that only returned whether the Node would respond to the events emitted by a tap gesture. The question here is: Is this something we would want to make generic so you can also tell if an element would respond to keyboard-events, etc? Or would it be better with a simpler touch-adjustment specific API?

[Ryosuke Niwa]

(In reply to comment #15)
> It is currently only used for touch-adjustment.

We already have needTouchEvents. We can't use needTouchEvents here?

[Antonio Gomes]

(In reply to comment #0)
> To complement the hasEventListener we should have a corresponding hasDefaultEventHandler that can tell if a node implementation will react in any way to an event passed to it.
> 
> This is useful in touch-adjustment which can more reliably tell which nodes might respond to a synthesised click event and which will node.

FWIW, in the blackberry implementation we list the nodes we are interested in filtering in, instead of asking webcore if it has a defaulthandle for the events we care about.

[Allan Sandfeld Jensen]

(In reply to comment #16)
> (In reply to comment #15)
> > It is currently only used for touch-adjustment.
> 
> We already have needTouchEvents. We can't use needTouchEvents here?

This is not about native touch events, this is asking if a node responds to click, DOMactivate, mouseup, mousedown or mousemove. The events that are emitted by a tap gesture.

(In reply to comment #17)
> (In reply to comment #0)
> > To complement the hasEventListener we should have a corresponding hasDefaultEventHandler that can tell if a node implementation will react in any way to an event passed to it.
> > 
> > This is useful in touch-adjustment which can more reliably tell which nodes might respond to a synthesised click event and which will node.
> 
> FWIW, in the blackberry implementation we list the nodes we are interested in filtering in, instead of asking webcore if it has a defaulthandle for the events we care about.

I am aware of that, and we had something similar in the browser for N9, but I want to move away from that kind of solution. Not only is such a list likely to be incomplete or get outdated. It is also expensive to match and can not solve the problem from bug 91894 with special shadow dom elements.

[Ryosuke Niwa]

(In reply to comment #18)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > It is currently only used for touch-adjustment.
> > 
> > We already have needTouchEvents. We can't use needTouchEvents here?
> 
> This is not about native touch events, this is asking if a node responds to click, DOMactivate, mouseup, mousedown or mousemove. The events that are emitted by a tap gesture.

My concern for adding this method will be that it'll be get out-of-date very quickly. e.g. how is a contributor supposed to know that he/she needs to update hasDefaultEventHandler when he/she has updated defaultEventHandler? Given that the vast majority of contributors come from Apple and Google these days and they don't use this feature, this is bound to regress.

I highly recommend to revise the approach you're taking here.

> I am aware of that, and we had something similar in the browser for N9, but I want to move away from that kind of solution. Not only is such a list likely to be incomplete or get outdated. It is also expensive to match and can not solve the problem from bug 91894 with special shadow dom elements.

How does moving the code into WebCore help that situation? I'm sorry but nobody is going to remember to update hasDefaultEventHandler in practice.

[Allan Sandfeld Jensen]

(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #16)
> > > (In reply to comment #15)
> > > > It is currently only used for touch-adjustment.
> > > 
> > > We already have needTouchEvents. We can't use needTouchEvents here?
> > 
> > This is not about native touch events, this is asking if a node responds to click, DOMactivate, mouseup, mousedown or mousemove. The events that are emitted by a tap gesture.
> 
> My concern for adding this method will be that it'll be get out-of-date very quickly. e.g. how is a contributor supposed to know that he/she needs to update hasDefaultEventHandler when he/she has updated defaultEventHandler? Given that the vast majority of contributors come from Apple and Google these days and they don't use this feature, this is bound to regress.

What kind of events a Node reacts to shouldn't change very often, most changes to defaultEventHandlers are how they react. Worst case scenario if it is out of date, is that taps are not adjusted correctly making the node slightly harder to hit for a user. 

Additionally it is possible to make watch-list for changes to defaultEventHandlers if we really want to monitor it.

> 
> I highly recommend to revise the approach you're taking here.
> 
> > I am aware of that, and we had something similar in the browser for N9, but I want to move away from that kind of solution. Not only is such a list likely to be incomplete or get outdated. It is also expensive to match and can not solve the problem from bug 91894 with special shadow dom elements.
> 
> How does moving the code into WebCore help that situation? I'm sorry but nobody is going to remember to update hasDefaultEventHandler in practice.

I am not moving anything into WebCore, touch-adjustment already is in WebCore. This is fixing a bug in WebCore. We can argue if this is the right way to fix it, but it has nothing to do with being in WebCore or not being in WebCore.

[Ryosuke Niwa]

(In reply to comment #20)
> (In reply to comment #19) 
> > How does moving the code into WebCore help that situation? I'm sorry but nobody is going to remember to update hasDefaultEventHandler in practice.
> 
> I am not moving anything into WebCore, touch-adjustment already is in WebCore. This is fixing a bug in WebCore. We can argue if this is the right way to fix it, but it has nothing to do with being in WebCore or not being in WebCore.

Then I'd argue that this is not the right fix. Additionally, you should reference to the bug you're trying to fix.

[Antonio Gomes]

> > I highly recommend to revise the approach you're taking here.
> > 
> > > I am aware of that, and we had something similar in the browser for N9, but I want to move away from that kind of solution. Not only is such a list likely to be incomplete or get outdated. It is also expensive to match and can not solve the problem from bug 91894 with special shadow dom elements.
> > 
> > How does moving the code into WebCore help that situation? I'm sorry but nobody is going to remember to update hasDefaultEventHandler in practice.
> 
> I am not moving anything into WebCore, touch-adjustment already is in WebCore. This is fixing a bug in WebCore. We can argue if this is the right way to fix it, but it has nothing to do with being in WebCore or not being in WebCore.

Sorry Allan, but I am in agreement with the others here. A list "fixed list" of elements won't change either, and can be a single place to keep this, imo.

[Allan Sandfeld Jensen]

(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #19) 
> > > How does moving the code into WebCore help that situation? I'm sorry but nobody is going to remember to update hasDefaultEventHandler in practice.
> > 
> > I am not moving anything into WebCore, touch-adjustment already is in WebCore. This is fixing a bug in WebCore. We can argue if this is the right way to fix it, but it has nothing to do with being in WebCore or not being in WebCore.
> 
> Then I'd argue that this is not the right fix. Additionally, you should reference to the bug you're trying to fix.

I didn't block the originally bug this patch could fix because I am not fully confident in whether this is the right solution or not either, but one way or another, those special nodes that can be tap-activated has to be recognizable, and they can not be matched by CSS queries, and they currently have no useful functions that can tell us if they can be tap-activated.

[Ryosuke Niwa]

(In reply to comment #23)
> I didn't block the originally bug this patch could fix because I am not fully confident in whether this is the right solution or not either, but one way or another, those special nodes that can be tap-activated has to be recognizable, and they can not be matched by CSS queries, and they currently have no useful functions that can tell us if they can be tap-activated.

I'm sorry but we don't normally add new functions to WebCore based on a possibility that it could help us moving forward in other bugs. Otherwise, we'll be adding all sorts of functions and classes that could be useful. Instead, we'll look at the actual problem/bug at hand, and then come up with a right solution for that problem along with a regression test.

As such, I don't think we should proceed with this patch. In fact, we should probably close this bug as WontFix and move onto the actual touch adjustment bug you mentioned.

[Allan Sandfeld Jensen]

(In reply to comment #24)
> I'm sorry but we don't normally add new functions to WebCore based on a possibility that it could help us moving forward in other bugs. Otherwise, we'll be adding all sorts of functions and classes that could be useful. Instead, we'll look at the actual problem/bug at hand, and then come up with a right solution for that problem along with a regression test.
> 
> As such, I don't think we should proceed with this patch. In fact, we should probably close this bug as WontFix and move onto the actual touch adjustment bug you mentioned.

Ey? I am sorry if I have offended you, but there is not reason to get confrontational. 

Bug 91894 can be solved in a more specific but incorrect way, which is the approach currently pursued since this correct solution might draw out with pointless discussions. Also bug 91894 is only one _example_ of the problem, the same problem happens in several instances. It is quite possible to extend this patch with several test-cases which it fixes. 

If we end up using a different approach, the title of the bug should be changed, but since I wanted to use the bug to post a possible solution for discussion, I kept it specific to the solution and not the problem.

[Allan Sandfeld Jensen]

Comment on attachment 154037 [details]
Patch

Lacks test-cases.

[Ryosuke Niwa]

(In reply to comment #25)
> (In reply to comment #24)
> > I'm sorry but we don't normally add new functions to WebCore based on a possibility that it could help us moving forward in other bugs. Otherwise, we'll be adding all sorts of functions and classes that could be useful. Instead, we'll look at the actual problem/bug at hand, and then come up with a right solution for that problem along with a regression test.
> > 
> > As such, I don't think we should proceed with this patch. In fact, we should probably close this bug as WontFix and move onto the actual touch adjustment bug you mentioned.
> 
> Ey? I am sorry if I have offended you, but there is not reason to get confrontational. 

I have no intention to be confrontational here. I'm just giving you information and stating my opinions.

> Bug 91894 can be solved in a more specific but incorrect way, which is the approach currently pursued since this correct solution might draw out with pointless discussions. Also bug 91894 is only one _example_ of the problem, the same problem happens in several instances. It is quite possible to extend this patch with several test-cases which it fixes. 

What are those other problems?

[Ryosuke Niwa]

Looking at the bug 91894, what we need to know is whether an element can be activated or not, which is a different question from whether an element has a default event handler or not.

[Allan Sandfeld Jensen]

(In reply to comment #28)
> Looking at the bug 91894, what we need to know is whether an element can be activated or not, which is a different question from whether an element has a default event handler or not.

True, but the detail is that we want to know if it will be activated (or in some other way react) to the events generated by a gesture tap. A gesture tap generates mousemove, mousedown and mouseup events. These events indirectly also triggers click, DOMActivate and focus events. 

We already have good ways of checking if an element is author programmed to react to these event using hasEventListener(eventType), and we can tell if it can be focused by calling is mouseFocusable(). The only part we do not know, is if something other than focus has an effect in the defaultEventHandler. So this first suggestion was to implement something similar to hasEventListener for the default event handling, that tells us if there is a chance the element will react to that event-type.

If we do not want something as generic as hasDefaultEventHandler(eventType), something else needs to tell us if the node/element will react to a click, DOMActivate, mousemove, mousedown or mouseup event in the defaultEventHandler.

I am open to suggestions.

[Ryosuke Niwa]

(In reply to comment #29)
> (In reply to comment #28)
> > Looking at the bug 91894, what we need to know is whether an element can be activated or not, which is a different question from whether an element has a default event handler or not.
> 
> True, but the detail is that we want to know if it will be activated (or in some other way react) to the events generated by a gesture tap. A gesture tap generates mousemove, mousedown and mouseup events. These events indirectly also triggers click, DOMActivate and focus events. 

I can't think of an element activation that can't be triggered by mousedown & mouse up. Given that, I tend to think that we don't need to differentiate elements that can be activated via a gesture tap and ones that can be activated by anything but a gesture tap.

> We already have good ways of checking if an element is author programmed to react to these event using hasEventListener(eventType), and we can tell if it can be focused by calling is mouseFocusable(). The only part we do not know, is if something other than focus has an effect in the defaultEventHandler. So this first suggestion was to implement something similar to hasEventListener for the default event handling, that tells us if there is a chance the element will react to that event-type.

The maintenance cost of hasEventListener is much no lower than hasDefaultEventHandler you proposed in previous patches because JSC and V8 know how many event listeners there are on a given element for a given event. On the other hand, maintaining that information for WebCore code incurs some cost, and we should try to minimize that.

One approach that might work, if we're going to take the original approach you took, then it's probably better to make defaultEventHandler answer the question when "event" (the argument) is 0.

> If we do not want something as generic as hasDefaultEventHandler(eventType), something else needs to tell us if the node/element will react to a click, DOMActivate, mousemove, mousedown or mouseup event in the defaultEventHandler.

We probably don't want that because there are default event handlers for all sorts of events, and we're only interested in whether an element activates or not.

[Ryosuke Niwa]

>One approach that might work, if we're going to take the original approach you took, then it's probably better to make defaultEventHandler answer the question when "event" (the argument) is 0.

should read

One approach that might work, if we're going to take the original approach you took, is to make defaultEventHandler answer the question when "event" (the argument) is 0.

[Benjamin Poulain]

I understand the problem you are trying to solve here, but in my opinion this patch is not a solution.

What you want is a way for an element to tell if it will, _in its current state_, respond to a click event. You must take into account the state of the element, and you don't need to support any type of events.

Even such code is hard to maintain. To make it more manageable, you should have it extensively tested (and ideally also used by Chromium for the synthetic click implementation).

[Allan Sandfeld Jensen]

(In reply to comment #32)
> I understand the problem you are trying to solve here, but in my opinion this patch is not a solution.
> 
> What you want is a way for an element to tell if it will, _in its current state_, respond to a click event. You must take into account the state of the element, and you don't need to support any type of events.
> 
> Even such code is hard to maintain. To make it more manageable, you should have it extensively tested (and ideally also used by Chromium for the synthetic click implementation).

So something like isMouseActivatable? 

I guess that could make sense, and matches the style of isMouseFocusable virtual functions.

[Ryosuke Niwa]

(In reply to comment #33)
> So something like isMouseActivatable? 

Yeah. That's probably the most sensible solution here.

[Benjamin Poulain]

> So something like isMouseActivatable?

Yep, that is the idea.
If you want to follow the iOS terminology: willRespondToMouseClickEvents().

[Allan Sandfeld Jensen]

(In reply to comment #35)
> > So something like isMouseActivatable?
> 
> Yep, that is the idea.
> If you want to follow the iOS terminology: willRespondToMouseClickEvents().

Well actually they seem to have set of these to cover various events 
willRespondToMouseClickEvents, willRespondToMouseMoveEvents, willRespondToMouseWheelEvents. I am not sure having multiple functions is any better than a generic one.

[Allan Sandfeld Jensen]

(In reply to comment #36)
> (In reply to comment #35)
> > > So something like isMouseActivatable?
> > 
> > Yep, that is the idea.
> > If you want to follow the iOS terminology: willRespondToMouseClickEvents().
> 
> Well actually they seem to have set of these to cover various events 
> willRespondToMouseClickEvents, willRespondToMouseMoveEvents, willRespondToMouseWheelEvents. I am not sure having multiple functions is any better than a generic one.

Okay I have decided to reuse the iOS terminology, but reimplement the functions.

I am implementing willRespondToMouseClickEvents and willRespondToMouseMoveEvents.

[Antonio Gomes]

> Okay I have decided to reuse the iOS terminology, but reimplement the functions.
> 
> I am implementing willRespondToMouseClickEvents and willRespondToMouseMoveEvents.

what elements are you planning to add willRespondeToMouseMoveEvents to? What event respending to mouse move event would not get catch in willRespondeToMouseClick?

Not sure both are needed, but I might be missing something...

[Allan Sandfeld Jensen]

(In reply to comment #38)
> > Okay I have decided to reuse the iOS terminology, but reimplement the functions.
> > 
> > I am implementing willRespondToMouseClickEvents and willRespondToMouseMoveEvents.
> 
> what elements are you planning to add willRespondeToMouseMoveEvents to? What event respending to mouse move event would not get catch in willRespondeToMouseClick?
> 
> Not sure both are needed, but I might be missing something...

The point is that these two functions not only tells of if the defaultEventHandler will react to the event but includes the check of hasEventListener.

The elements that would be missing by only using willRespondToMouseClick are those that react to mousemove events, which in most cases will be those with an eventlistener on mousemove.

[Antonio Gomes]

(In reply to comment #39)
> (In reply to comment #38)
> > > Okay I have decided to reuse the iOS terminology, but reimplement the functions.
> > > 
> > > I am implementing willRespondToMouseClickEvents and willRespondToMouseMoveEvents.
> > 
> > what elements are you planning to add willRespondeToMouseMoveEvents to? What event respending to mouse move event would not get catch in willRespondeToMouseClick?
> > 
> > Not sure both are needed, but I might be missing something...
> 
> The point is that these two functions not only tells of if the defaultEventHandler will react to the event but includes the check of hasEventListener.
> 
> The elements that would be missing by only using willRespondToMouseClick are those that react to mousemove events, which in most cases will be those with an eventlistener on mousemove.

Makes sense.

[Allan Sandfeld Jensen]

Created attachment 154934 [details]
Patch

[Kenneth Rohde Christiansen]

I think this is a pretty okay way of doing it

[Antonio Gomes]

Comment on attachment 154934 [details]
Patch

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

> Source/WebCore/dom/Node.h:577
> +    virtual bool willRespondToMouseMoveEvents();
> +    virtual bool willRespondToMouseClickEvents();

'const' here and throughout?

[Allan Sandfeld Jensen]

(In reply to comment #43)
> (From update of attachment 154934 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154934&action=review
> 
> > Source/WebCore/dom/Node.h:577
> > +    virtual bool willRespondToMouseMoveEvents();
> > +    virtual bool willRespondToMouseClickEvents();
> 
> 'const' here and throughout?

Good idea, I will add that with the next version. I am just adding a few test-cases.

[Allan Sandfeld Jensen]

Created attachment 154953 [details]
Patch

[Allan Sandfeld Jensen]

(In reply to comment #44)
> (In reply to comment #43)
> > (From update of attachment 154934 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=154934&action=review
> > 
> > > Source/WebCore/dom/Node.h:577
> > > +    virtual bool willRespondToMouseMoveEvents();
> > > +    virtual bool willRespondToMouseClickEvents();
> > 
> > 'const' here and throughout?
> 
> Good idea, I will add that with the next version. I am just adding a few test-cases.

No, sorry. I forgot that hasEventListeners() is non-const for some reason, so these functions will have to be non-const as well.

[Allan Sandfeld Jensen]

This patch willl need to be rebased before landing since the patch for bug #91894 landed, but it should still be suitable for reviewing.

[Antonio Gomes]

Comment on attachment 154953 [details]
Patch

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

nice!

r=me

> Source/WebCore/ChangeLog:11
> +        Touch-adjustment is changed to using the two functions which improves accuracy when

use?

[Allan Sandfeld Jensen]

Committed r124022: <http://trac.webkit.org/changeset/124022>

[Adam Barth]

Comment on attachment 154953 [details]
Patch

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

> LayoutTests/touchadjustment/disabled-formelements.html:48
> +//         adjustedNode = testRoundTouch(16, 8, 5);
> +//         shouldBeEqualToString('adjustedNode.id', 'label1');

Did you intend to land this with commented out test code?

[Allan Sandfeld Jensen]

(In reply to comment #49)
> Committed r124022: <http://trac.webkit.org/changeset/124022>

(In reply to comment #50)
> (From update of attachment 154953 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154953&action=review
> 
> > LayoutTests/touchadjustment/disabled-formelements.html:48
> > +//         adjustedNode = testRoundTouch(16, 8, 5);
> > +//         shouldBeEqualToString('adjustedNode.id', 'label1');
> 
> Did you intend to land this with commented out test code?

Yes, I forgot to comment on that. They are to be un-commented once bug #91012 lands. I thought it might land before this one.

[Allan Sandfeld Jensen]

(In reply to comment #51)
> (In reply to comment #49)
> > Committed r124022: <http://trac.webkit.org/changeset/124022>
> 
> (In reply to comment #50)
> > (From update of attachment 154953 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=154953&action=review
> > 
> > > LayoutTests/touchadjustment/disabled-formelements.html:48
> > > +//         adjustedNode = testRoundTouch(16, 8, 5);
> > > +//         shouldBeEqualToString('adjustedNode.id', 'label1');
> > 
> > Did you intend to land this with commented out test code?
> 
> Yes, I forgot to comment on that. They are to be un-commented once bug #91012 lands. I thought it might land before this one.

Oh wait, never mind. I don't think I thought that through. I will remove them when I do the upcoming renovation on touchadjustment tests.