Bug 193478 - [Mac] Add a new quirk to HTMLFormControlElement::isMouseFocusable
Summary: [Mac] Add a new quirk to HTMLFormControlElement::isMouseFocusable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-15 18:22 PST by Jiewen Tan
Modified: 2019-01-17 13:30 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.03 KB, patch)
2019-01-15 18:31 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (3.55 KB, patch)
2019-01-16 11:58 PST, Jiewen Tan
bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (3.63 KB, patch)
2019-01-17 11:08 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2019-01-15 18:22:56 PST
Add a new quirk to HTMLFormControlElement::isMouseFocusable such that submit buttons are mouse focusable. This quirk is for ceac.state.gov specifically.
Comment 1 Jiewen Tan 2019-01-15 18:24:44 PST
<rdar://problem/34368591>
Comment 2 Jiewen Tan 2019-01-15 18:31:21 PST
Created attachment 359238 [details]
Patch
Comment 3 Jiewen Tan 2019-01-15 18:32:45 PST
Comment on attachment 359238 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.cpp:661
> +

The new line should be deleted.
Comment 4 Chris Dumez 2019-01-15 19:19:33 PST
Comment on attachment 359238 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.cpp:662
> +bool HTMLFormControlElement::needsSiteSpecificQuirks() const

The name should say what the quirk is about. What if we had more than one quirk?
Comment 5 Chris Dumez 2019-01-15 19:19:36 PST
Comment on attachment 359238 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.cpp:662
> +bool HTMLFormControlElement::needsSiteSpecificQuirks() const

The name should say what the quirk is about. What if we had more than one quirk?
Comment 6 Chris Dumez 2019-01-15 19:34:19 PST
Comment on attachment 359238 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Add a new quirk to HTMLFormControlElement::isMouseFocusable such that submit buttons are mouse focusable.

Could you explain in the changelog why we do this behavior only for this site and not all sites? How do other browsers behave?

>>> Source/WebCore/html/HTMLFormControlElement.cpp:662
>>> +bool HTMLFormControlElement::needsSiteSpecificQuirks() const
>> 
>> The name should say what the quirk is about. What if we had more than one quirk?
> 
> The name should say what the quirk is about. What if we had more than one quirk?

E.g. needsMouseFocusableQuirk()
Comment 7 Jiewen Tan 2019-01-16 11:55:12 PST
Comment on attachment 359238 [details]
Patch

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

Thanks Chris for reviewing the patch.

>> Source/WebCore/ChangeLog:9
>> +        Add a new quirk to HTMLFormControlElement::isMouseFocusable such that submit buttons are mouse focusable.
> 
> Could you explain in the changelog why we do this behavior only for this site and not all sites? How do other browsers behave?

Sure, improved.

>>>> Source/WebCore/html/HTMLFormControlElement.cpp:662
>>>> +bool HTMLFormControlElement::needsSiteSpecificQuirks() const
>>> 
>>> The name should say what the quirk is about. What if we had more than one quirk?
>> 
>> The name should say what the quirk is about. What if we had more than one quirk?
> 
> E.g. needsMouseFocusableQuirk()

Hopefully not. Fixed.
Comment 8 Jiewen Tan 2019-01-16 11:58:50 PST
Created attachment 359287 [details]
Patch
Comment 9 Jiewen Tan 2019-01-16 16:47:56 PST
Comment on attachment 359287 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.cpp:660
>  

Added: // FIXME: We should remove the quirk once <rdar://problem/47334655> is fixed.
Comment 10 Brent Fulgham 2019-01-16 19:48:26 PST
Comment on attachment 359287 [details]
Patch

Thanks!
Comment 11 Jiewen Tan 2019-01-17 11:08:30 PST
Created attachment 359392 [details]
Patch for landing
Comment 12 Jiewen Tan 2019-01-17 11:08:54 PST
(In reply to Brent Fulgham from comment #10)
> Comment on attachment 359287 [details]
> Patch
> 
> Thanks!

Thanks for r+ it.
Comment 13 WebKit Commit Bot 2019-01-17 11:34:34 PST
Comment on attachment 359392 [details]
Patch for landing

Clearing flags on attachment: 359392

Committed r240122: <https://trac.webkit.org/changeset/240122>