Validity property not immediately updated when Input change

RESOLVED FIXED in Firefox 66

Status

()

defect
P2
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: znarfor, Assigned: masayuki)

Tracking

({regression})

Trunk
mozilla67
All
Unspecified
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 wontfix, firefox66 fixed, firefox67 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 months ago

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

4 months ago

I can reproduce this bug on Firefox 65 / Ubuntu.

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

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.

Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Core & HTML
Ever confirmed: true
Product: Firefox → Core
Hardware: Unspecified → All
Version: 65 Branch → Trunk

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

Flags: needinfo?(stefan.georgiev)

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!

Flags: needinfo?(stefan.georgiev) → needinfo?(masayuki)

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

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)

"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.

Priority: -- → P2

Comment 9

3 months ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/b880dbaefb04
Make nsContentUtils::DispatchInputEvent() update HTMLInputElement.validationMessage before dispatching "input" event r=smaug

Comment 10

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

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.

Attachment #9042419 - Flags: approval-mozilla-beta?

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.

Attachment #9042419 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

3 months ago
Blocks: 1533811

Updated

3 months ago
No longer blocks: 1533811
Duplicate of this bug: 1533811

Updated

2 months ago
Depends on: 1539172
No longer depends on: 1539172
Regressions: 1539172
You need to log in before you can comment on or make changes to this bug.