Bug 193046 – Unable to enter text in https://eat.fi

• Component: Forms
• Status: RESOLVED FIXED

[Alberto Garcia]

I reproduced this with the MiniBrowser using WebKitGTK+ 2.22.5.

1) Open https://eat.fi/
2) When the page is loaded look for an input box that says "Hae kaupunki tai ravintola" ("Search city or restaurant")
3) Click on the input box with the mouse in order to set the focus.

Expected outcome: the input box is focused and you can type some text.
Actual outcome: the form is submitted automatically before you can type anything.

This works fine in Chromium and Firefox.

[Adrian Perez]

This is still happening in 2.24.1 :-⅜

[Lionir]

Thought I'd add, if you tab to the search bar and type in it, it works. So it's likely how we grab focus on the search bar which causes the problem?

[Carlos Garcia Campos]

This is not GTK specific, it happens in Safari too.

[Carlos Garcia Campos]

Created attachment 374599 [details]
Patch

[Michael Catanzaro]

Nice!

[Carlos Garcia Campos]

Ping reviewers

[Darin Adler]

The failure in this test on iOS seems like it might be a real regression:

imported/blink/fast/forms/label/label-contains-other-interactive-content.html

Has anyone looked at it yet?

[Carlos Garcia Campos]

Well, it's a new test, so it's not actually a regression, I would say. But I don't know iOS to understand why it's only failing there.

[Darin Adler]

We will need an iOS expert to help with that maybe.

[Carlos Garcia Campos]

Could it be that clickElement() doesn't work for some reason in iOS WTR? I see other tests using clickElement listed in ios-wk2/TestExpectations, but without a bug number nor more details. I think we can just skip this test in iOS for now.

[Carlos Garcia Campos]

That's it, mouseDown(), mouseUp() and mouseMoveTo() are not implemented in iOS EventSender.

[Carlos Garcia Campos]

Created attachment 375219 [details]
Patch

[Carlos Garcia Campos]

New test failure is unrelated to this, it's the same partch as before, just with the test skipped in iOS.

[Ryosuke Niwa]

Comment on attachment 375219 [details]
Patch

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

r=me with the following comments addressed.

> Source/WebCore/html/HTMLEmbedElement.h:46
> +    bool isInteractiveContent() const final { return true; }

Please add this to HTMLAppletElement as well.

> Source/WebCore/html/HTMLIFrameElement.h:56
> +    bool isInteractiveContent() const final { return true; }

And to HTMLFrameElement.

> Source/WebCore/html/HTMLLabelElement.cpp:123
> +    for (const auto* it = &node; it && it != this; it = it->parentOrShadowHostNode()) {

Use parentElement() or parentElementInComposedTree().
It's suspicious that the specification doesn't use the composed tree for the purpose of determining this.
It's probably a specification bug.

[Carlos Garcia Campos]

(In reply to Ryosuke Niwa from comment #14)
> Comment on attachment 375219 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375219&action=review
> 
> r=me with the following comments addressed.

Thanks for the review.

> > Source/WebCore/html/HTMLEmbedElement.h:46
> > +    bool isInteractiveContent() const final { return true; }
> 
> Please add this to HTMLAppletElement as well.

Ok.

> > Source/WebCore/html/HTMLIFrameElement.h:56
> > +    bool isInteractiveContent() const final { return true; }
> 
> And to HTMLFrameElement.

Why? The spec only mentions iframe.

> > Source/WebCore/html/HTMLLabelElement.cpp:123
> > +    for (const auto* it = &node; it && it != this; it = it->parentOrShadowHostNode()) {
> 
> Use parentElement() or parentElementInComposedTree().
> It's suspicious that the specification doesn't use the composed tree for the
> purpose of determining this.
> It's probably a specification bug.

Ok, I'll use parentElementInComposedTree()

[Carlos Garcia Campos]

Created attachment 377331 [details]
Patch for landing

[Carlos Garcia Campos]

Committed r249194: <https://trac.webkit.org/changeset/249194>

[Radar WebKit Bug Importer]

<rdar://problem/54784134>

[Ryosuke Niwa]

(In reply to Carlos Garcia Campos from comment #15)
> (In reply to Ryosuke Niwa from comment #14)
> > Comment on attachment 375219 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=375219&action=review
> > 
> > r=me with the following comments addressed.
> 
> Thanks for the review.
>
> > > Source/WebCore/html/HTMLIFrameElement.h:56
> > > +    bool isInteractiveContent() const final { return true; }
> > 
> > And to HTMLFrameElement.
> 
> Why? The spec only mentions iframe.

That's probably just a bug in the spec itself but whatever. It doesn't matter for now.