1318339 - [DateTimeInput] When action on Delete button an " | " is displayed over it

Status: VERIFIED FIXED

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

Description

[roxleitan]

Attachment: time input.html

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20161115030213

[Affected versions]:
Nightly 53.0a1

[Affected platforms]:
All platforms: Windows 10, Windows 7, Ubuntu 16.04

[Steps to reproduce]:
1.Launch Nightly 53.0a1 with a new profile
2.Set "dom.forms.datetime" pref to True in about:config
3.Open attached html
4.Press Tab key to navigate through input columns (hour, minutes, AM/PM)
5.Click on Delete button


[Expected result]:
Values from hour and minutes should be deleted. Delete button should be displayed correctly.

[Actual result]:
Values from hour and minutes are deleted, but over the Delete button is displayed a vertical line.

[Additional notes]:
Ubuntu: The issue is reproducible without step 4.

Comment 1

[roxleitan]

Attachment: Delete button.png

Comment 2

[Jessica Jong [:jessica] (inactive)]

Hi Roxana, is this bug still valid? as we now hide the reset button when there are no values.

Comment 3

[roxleitan]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170305030210

Hi Jessica, the issue is still reproducible even if Delete button appears only after time values are set.
Reproducible only on Windows and Ubuntu.

Comment 4

[Jessica Jong [:jessica] (inactive)]

Attachment: Bug 1318339 - [DateTimeInput] Prevent reset button from getting focus.

Reset button should not be tabbable or focusable.

Review commit: https://reviewboard.mozilla.org/r/132354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132354/

Comment 5

[Jessica Jong [:jessica] (inactive)]

Hi :mossop, here comes another one, this one is simpler. :)

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 8860324 [details]
Bug 1318339 - [DateTimeInput] Prevent reset button from getting focus.

https://reviewboard.mozilla.org/r/132354/#review135354

Can you elaborate more about what's going on here? My understanding of what the patch is doing doesn't seem to match the commit message.

Comment 7

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8860324 [details]
Bug 1318339 - [DateTimeInput] Prevent reset button from getting focus.

https://reviewboard.mozilla.org/r/132354/#review135354

Sorry, should have made myself clearer. There is a dotted line on the button because it's being focused. We can either fix this by removing the border of the button on focus (on its stylesheet), or prevent the button for getting focus at all. I chose the latter one, because I think the button should not steal the focus from the text field, even if the focus will be set back to the first inner text box.

Comment 8

[Dave Townsend [:mossop]]

(In reply to Jessica Jong [:jessica] from comment #7)
> Comment on attachment 8860324 [details]
> Bug 1318339 - [DateTimeInput] Prevent reset button from getting focus.
> 
> https://reviewboard.mozilla.org/r/132354/#review135354
> 
> Sorry, should have made myself clearer. There is a dotted line on the button
> because it's being focused. We can either fix this by removing the border of
> the button on focus (on its stylesheet), or prevent the button for getting
> focus at all. I chose the latter one, because I think the button should not
> steal the focus from the text field, even if the focus will be set back to
> the first inner text box.

Ok and I can buy that the change makes it so you can't focus with the mouse, but how does it stop it being focused by keyboard? Would doing that be an accessibility issue?

Comment 9

[Jessica Jong [:jessica] (inactive)]

(In reply to Dave Townsend [:mossop] from comment #8)
> Ok and I can buy that the change makes it so you can't focus with the mouse,
> but how does it stop it being focused by keyboard? Would doing that be an
> accessibility issue?

Oh, that has already been done with `tabindex=-1` [1]. Hmm, I'm not sure about accessibility, but it seems that the up/down keys in input type=number or the reset button in input type=date/time on Chrome browser are not tabbable either.

[1] http://searchfox.org/mozilla-central/rev/baf47b352e873d4516d7656186d6d7c7447d3873/toolkit/content/widgets/datetimebox.xml#1095

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8860324 [details]
Bug 1318339 - [DateTimeInput] Prevent reset button from getting focus.

https://reviewboard.mozilla.org/r/132354/#review136438

Comment 11

[Jessica Jong [:jessica] (inactive)]

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc40abcbe193
[DateTimeInput] Prevent reset button from getting focus. r=mossop

Comment 13

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

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

Comment 14

[roxleitan]

Verified as fixed using the latest Nightly 55.0a1 (2017-05-05) on Ubuntu 16.04 x64, Windows 10 x64 and Mac OS X 10.12.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

I think we can assume 54 is affected here. Want to uplift this fix to 54?

Comment 16

[Jessica Jong [:jessica] (inactive)]

I don't think this needs to be uplifted, it's a minor issue and it's pref-off by default.