Bug 193046 - Unable to enter text in https://eat.fi
Summary: Unable to enter text in https://eat.fi
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-28 02:05 PST by Alberto Garcia
Modified: 2019-08-28 08:00 PDT (History)
11 users (show)

See Also:


Attachments
Patch (21.38 KB, patch)
2019-07-22 07:19 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (22.25 KB, patch)
2019-07-31 01:31 PDT, Carlos Garcia Campos
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (22.77 KB, patch)
2019-08-27 02:03 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2018-12-28 02:05:23 PST
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.
Comment 1 Adrian Perez 2019-04-25 14:52:36 PDT
This is still happening in 2.24.1 :-⅜
Comment 2 Lionir 2019-05-17 13:25:19 PDT
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?
Comment 3 Carlos Garcia Campos 2019-07-22 07:07:25 PDT
This is not GTK specific, it happens in Safari too.
Comment 4 Carlos Garcia Campos 2019-07-22 07:19:31 PDT
Created attachment 374599 [details]
Patch
Comment 5 Michael Catanzaro 2019-07-22 12:46:35 PDT
Nice!
Comment 6 Carlos Garcia Campos 2019-07-29 02:08:30 PDT
Ping reviewers
Comment 7 Darin Adler 2019-07-29 10:07:18 PDT
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?
Comment 8 Carlos Garcia Campos 2019-07-30 06:08:44 PDT
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.
Comment 9 Darin Adler 2019-07-30 14:19:02 PDT
We will need an iOS expert to help with that maybe.
Comment 10 Carlos Garcia Campos 2019-07-31 01:19:26 PDT
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.
Comment 11 Carlos Garcia Campos 2019-07-31 01:23:50 PDT
That's it, mouseDown(), mouseUp() and mouseMoveTo() are not implemented in iOS EventSender.
Comment 12 Carlos Garcia Campos 2019-07-31 01:31:40 PDT
Created attachment 375219 [details]
Patch
Comment 13 Carlos Garcia Campos 2019-08-01 08:32:00 PDT
New test failure is unrelated to this, it's the same partch as before, just with the test skipped in iOS.
Comment 14 Ryosuke Niwa 2019-08-21 17:26:34 PDT
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.
Comment 15 Carlos Garcia Campos 2019-08-27 02:02:45 PDT
(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()
Comment 16 Carlos Garcia Campos 2019-08-27 02:03:28 PDT
Created attachment 377331 [details]
Patch for landing
Comment 17 Carlos Garcia Campos 2019-08-28 03:23:03 PDT
Committed r249194: <https://trac.webkit.org/changeset/249194>
Comment 18 Radar WebKit Bug Importer 2019-08-28 03:24:21 PDT
<rdar://problem/54784134>
Comment 19 Ryosuke Niwa 2019-08-28 08:00:17 PDT
(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.