Synthesizing a click() on input type=date should not show the date picker UI

RESOLVED FIXED in Firefox 60

Status

()

defect
P2
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: miketaylr, Assigned: smaug)

Tracking

60 Branch
mozilla61
Points:
---
Bug Flags:
webcompat +
in-testsuite +

Firefox Tracking Flags

(firefox60 fixed, firefox61 fixed)

Details

(Whiteboard: [webcompat:p1], URL)

User Story

Business driver: Achieve tier-1 Google Search experience for Gecko on Android

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
Per https://github.com/whatwg/html/issues/3232, Firefox is the only browser that allows dispatching a synthetic click event to open the date picker UI:

Here's a simple test case:

https://output.jsbin.com/sonopow/quiet

We should match the other major browsers behavior here, for interop.
(Reporter)

Updated

a year ago
Flags: webcompat+
Whiteboard: [webcompat:p2]
(Reporter)

Comment 1

a year ago
Setting to [webcompat:p1] because this affects Google Tier 1 Search.
Whiteboard: [webcompat:p2] → [webcompat:p1]
Priority: -- → P2
(Assignee)

Updated

a year ago
Component: DOM: Events → DOM: Core & HTML
(Assignee)

Comment 2

a year ago
This is trivial, at least on desktop. And doesn't seem to apply to mobile.
Assignee: nobody → bugs
(Assignee)

Comment 3

a year ago
Posted patch date_click.diff (obsolete) — Splinter Review
This is a very small patch :)

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/2bc9d7050b66ef4d34abccefcc953f0aff3e4e2b
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bc9d7050b66ef4d34abccefcc953f0aff3e4e2b
remote: recorded changegroup in replication log in 0.095s
Attachment #8963725 - Flags: review?(mconley)
Comment on attachment 8963725 [details] [diff] [review]
date_click.diff

Review of attachment 8963725 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: toolkit/content/widgets/datetimebox.xml
@@ +1255,5 @@
>          // Handle keypress separately since we need to catch it on capturing.
>          this.addEventListener("keypress", this, {
>            capture: true,
>            mozSystemGroup: true
> +        },

Nit - I think it's usually more common to have these on the same line, like:

this.addEventListener("keypress", this, {
  capture: true,
  mozSystemGroup: true,
}, false);
Attachment #8963725 - Flags: review?(mconley) → review+
(Assignee)

Comment 5

a year ago

Comment 6

a year ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/223010e3a593
Synthesizing a click() on input type=date should not show the date picker UI , r=mconley

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/223010e3a593
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Should we get this on 60 to avoid webcompat pain on esr?
Flags: needinfo?(bugs)
(Assignee)

Comment 9

a year ago
I don't know. The patch should be quite safe anyhow.

Mike may have an opinion.
Flags: needinfo?(bugs) → needinfo?(miket)
(Reporter)

Comment 10

a year ago
Yes, it seems like a good idea, if possible.
Flags: needinfo?(miket)
(Assignee)

Comment 11

a year ago
ok, let me try whether the patches apply to 60
(Assignee)

Comment 12

a year ago
There doesn't seem to have a flag yet for esr60 approval.

The patch itself seems to apply.
Flags: needinfo?(jcristau)
There is, it's called "approval-mozilla-beta" :)
Flags: needinfo?(jcristau)
(Assignee)

Comment 14

a year ago
Comment on attachment 8963821 [details] [diff] [review]
date_click_2.diff

Approval Request Comment
[Feature/Bug causing the regression]: This has been an issue since we added support for type=date
[User impact if declined]: different behavior than in other browsers
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes by tests
[Needs manual test from QE? If yes, steps to reproduce]: I don't think manual test is needed 
[List of other uplifts needed for the feature/fix]: NA
[Is the change risky?]: shouldn't be too risky.
[Why is the change risky/not risky?]: we end up handling trusted-only events like other similar element implementations
[String changes made/needed]: NA
Attachment #8963821 - Flags: approval-mozilla-beta?
Comment on attachment 8963821 [details] [diff] [review]
date_click_2.diff

let's get this in 60.0b11
Attachment #8963821 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8963725 - Attachment is obsolete: true
(Reporter)

Updated

a year ago
User Story: (updated)
You need to log in before you can comment on or make changes to this bug.