1371111 - Clicking input[type=date] padding does not open the chooser

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Jan Tojnar]

Attachment: Areas of the date input

How to reproduce:
Click on the padding area of an input[type=date] (highlighted blue in the attached image).

What should happen:
First subfield should be selected and date chooser should be opened, just like when clicking the empty part of the content box (highlighted red).

What is happening:
First subfield (month) is selected.

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[triage]
P2.

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v1.

Hi Mike, could you take a look at this tiny patch? :)

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8897693 [details] [diff] [review]
patch, v1.

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

Thanks! This looks good, but I have some minor questions - see below.

-'ing until I hear back.

::: toolkit/content/widgets/datetimebox.xml
@@ +1236,5 @@
>            mozSystemGroup: true
>          });
> +        // This is to open the picker when input element's padding area is
> +        // clicked.
> +        this.mInputElement.addEventListener("click", this,

Should we be removing this (and the blur listener) in the destructor?

Also, since we're listening to the "click" event here, can we safely remove "click" from EVENTS?

Comment 4

[Jessica Jong [:jessica] (inactive)]

(In reply to Mike Conley (:mconley) from comment #3)
> Comment on attachment 8897693 [details] [diff] [review]
> patch, v1.
> 
> Review of attachment 8897693 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks! This looks good, but I have some minor questions - see below.
> 
> -'ing until I hear back.
> 
> ::: toolkit/content/widgets/datetimebox.xml
> @@ +1236,5 @@
> >            mozSystemGroup: true
> >          });
> > +        // This is to open the picker when input element's padding area is
> > +        // clicked.
> > +        this.mInputElement.addEventListener("click", this,
> 
> Should we be removing this (and the blur listener) in the destructor?

Yes, how did I forget about them. :/

> 
> Also, since we're listening to the "click" event here, can we safely remove
> "click" from EVENTS?

Yes, we can, since the click event on the xul element eventually bubbles up to the input element. Thanks for the suggestion.

Comment 5

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v2.

Addressed review comment 3.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8898593 [details] [diff] [review]
patch, v2.

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

Thanks!

Comment 7

[Jessica Jong [:jessica] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98eb437f623caa4772b407b98ed636fc759e2312&group_state=expanded

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fecc3b17395a
Open picker when input element's padding area is clicked. r=mconley

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fecc3b17395a