1524212 - Validity property not immediately updated when Input change

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[François Hodierne]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

To my knowledge, this is only reproducible on Mac OS.

1. Go on: https://jsfiddle.net/2x514d8m/7/
2. Fill the email input using Firefox suggestion
   a) Type the first letter of an usual email
   b) Use one of the suggestions from Firefox

Actual results:

The validity API reports that the email is invalid.

validationMessage="Please enter an email address."

Note that checking the validity API slightly later would work.

setTimeout(() => { console.log(event.target.validationMessage); }, 1)

Expected results:

The validity API should report that the email is valid.

validationMessage=""

Comment 1

[benjamin.piouffle]

I can reproduce this bug on Firefox 65 / Ubuntu.

My previous version, Firefox 64, was not affected by this issue.

Comment 2

[Stefan [:StefanG_QA]]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0 (20190205023948)

I've tested this report on Windows 10 and Ubuntu 18.10 using the latest Nightly, Beta and Fx release build. I was able to reproduce the mentioned behavior across different channels. When entering my email from the browser suggestions, my email address is not validated as it should be. This issue is not reproducible on the competitor browser.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Stefan,
Comment 1 seems suggesting this regressed in 65. Can you help find a regression window? Thanks!

Comment 4

[Stefan [:StefanG_QA]]

Here are my findings:
Last good revision: fbebc15cd4f40f0978bf22def0fcf355ce69f27b
First bad revision: 824bcd08c85e70d98109b04715b26f139e099ce8

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fbebc15cd4f40f0978bf22def0fcf355ce69f27b&tochange=824bcd08c85e70d98109b04715b26f139e099ce8

Can you take a look at this regression range? Thanks!

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://phabricator.services.mozilla.com/D12245
This is the cause of this regression. I keep investigating...

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cebe5b8e6daa6dabbdcc5ae96a0a0347170c50de

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I hope that usual cases should be covered by WPT:
https://searchfox.org/mozilla-central/search?q=.validationMessage&case=false&regexp=false&path=web-platform
https://searchfox.org/mozilla-central/search?q=.validity&case=false&regexp=false&path=web-platform

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1524212 - Make nsContentUtils::DispatchInputEvent() update HTMLInputElement.validationMessage before dispatching "input" event

"input" event listener may want to check HTMLInputElement.validationMessage.
However, due to moving "input" event dispatcher from
HTMLInputElement::SetUserInput() to editor, HTMLInputElement::SetValueInternal()
updates it after dispatching "input" event.

This patch makes nsContentUtils::DispatchInputEvent() guarantees to update
validationMessage value before dispatching every event. On the other hand,
SetValueInternal() may be called without "input" event dispatchers. Therefore,
it needs to keep updating validationMessage value in such cases.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b880dbaefb04
Make nsContentUtils::DispatchInputEvent() update HTMLInputElement.validationMessage before dispatching "input" event r=smaug

Comment 10

[Natalia Csoregi [:nataliaCs]]

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

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdcdc25d95896ded7ba05646fa219d8b17c5793e

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9042419 [details]
Bug 1524212 - Make nsContentUtils::DispatchInputEvent() update HTMLInputElement.validationMessage before dispatching "input" event

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1504911

User impact if declined

If web apps updates enable/disable state of submit button for an email input field and user selects an address with autocomplte, such button may be keep disabled even thought the selected value is valid email address.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Adding updater of validation state immediately before dispatching "input" events. Additionally, keep doing it when its value is changed by JS or at initialization.

String changes made/needed

None.

Comment 13

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

Comment on attachment 9042419 [details]
Bug 1524212 - Make nsContentUtils::DispatchInputEvent() update HTMLInputElement.validationMessage before dispatching "input" event

Fix for certain conditions for email validation, verified in nightly.
Thanks for the new tests!
OK for uplift for beta 8.

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/753ea9ad88e0