AngularJS date input not showing initial value inside directives in Firefox

RESOLVED FIXED in Firefox 65

Status

()

defect
P2
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: ryanmk54, Assigned: timdream)

Tracking

({regression})

65 Branch
mozilla66
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65+ fixed, firefox66+ fixed)

Details

()

Attachments

(1 attachment)

Reporter

Description

5 months ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

Here is the issue on StackOverflow: https://stackoverflow.com/questions/53768498/angularjs-date-input-not-showing-initial-value-inside-directives-in-firefox

Here is a code pen to demonstrate the issue: https://codepen.io/anon/pen/QzydRa

It works just fine on Firefox 64, but it doesn't work on 65.0b4. I have an AngularJS date input that is initialized properly inside the top level view with a controller, but it isn't if it is inside a directive.


Actual results:

The model is updated on change, but it isn't initialized properly. 


Expected results:

It should show the initial value from the model

Comment 1

5 months ago
Reproducible on Firefox Nightly 66.0a1 (2018-12-17), Firefox 65.0b4 on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.13.
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: Form Controls
Ever confirmed: true
Product: Firefox → Core
Given how widely used Angular is, this seems like a P2.
Priority: -- → P2
INFO: Last good revision: e9ac4160d051f7ec520c7a0534b75c4b10386873
INFO: First bad revision: 4ec5f59ab7236bce89f734ccdea1ec4c7f5fd95d
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e9ac4160d051f7ec520c7a0534b75c4b10386873&tochange=4ec5f59ab7236bce89f734ccdea1ec4c7f5fd95d
Blocks: 1496242
Has Regression Range: --- → yes
Flags: needinfo?(timdream)
:-'(
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
This bug happens because the UA Widget gets its MozDateTimeValueChanged event from the layout frame. Angular sets the |value| of the input before making it visible. I can reproduce the same bug with the following:

data:text/html,<input type="date" hidden><script>onload = () => { el = document.body.firstChild; el.value = "2019-02-28"; el.hidden = false; el.scrollTop; }</script>

You can workaround this bug by doing it the other way around

data:text/html,<input type="date" hidden><script>onload = () => { el = document.body.firstChild; el.hidden = false; el.scrollTop; el.value = "2019-02-28";  }</script>

This won't happen to the XBL binding because it constructs with the frame. The UI will construct with the correct value.
The XBL binding implementation relied on nsDateTimeControlFrame to call into
its nsIDateTimeInputArea implementation. This is correct because the XBL binding
is only constructed when the element has a frame. If the value is set while the
element is hidden, the XBL binding will pick up the correct value during construction.

That is not the case for UA Widget. As it is constructed when the DOM is attached,
relying on nsDateTimeControlFrame to send an event when attributes change means
the event won't be sent to the already constructed UA Widget.

This patch fix that by moving the event dispatching calls originating from
HTMLInputElement out of nsDateTimeControlFrame, so they will hehave correctly in
absence of the frame.

Sadly this means the XBL implementation and the UA Widget implementation are further
diveraged. The complexity should go away when we could finally remove the XBL implementation.

nsDateTimeControlFrame still dispatches a few events to UA Widget, in
AttributeChanged() and SyncDisabledState(), as they are originated from the layout.

The name of the events in AttributeChanged() are incorrect though -- I am correcting
that in this patch too.
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #6)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=abd9d24a0001409f66c093f5f5e40b05a4ac270c

Need fixing...

Comment 10

5 months ago
Pushed by tchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6dd228164d42
Dispatch events to hidden datetimebox UA Widget r=smaug

Comment 11

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6dd228164d42
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Why does this use `ChromeOnlyDispatch::eNo`? Does this accidentally expose these events to content? Maybe not since we're in Shadow DOM, but it may be better to pass yes regardless?
Flags: needinfo?(timdream)
We are dispatching the events to Shadow DOM. I don't think UA Widget can hear the event if you pass yes there.
Flags: needinfo?(timdream)
Need uplift
Flags: needinfo?(timdream)

Comment on attachment 9034041 [details]
Bug 1514040 - Dispatch events to hidden datetimebox UA Widget

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1496242

User impact if declined: <input type=date> will behave incorrectly if it is hidden when inserted to the DOM.

Is this code covered by automated tests?: No

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): The LOC changes look sizable but it simply only involves moving code between classes. Their behaviors are covered in automation. I didn't add a new test to test insert-to-DOM-while-hidden because I don't think this bug by itself will likely regress by future changes.

String changes made/needed:

Flags: needinfo?(timdream)
Attachment #9034041 - Flags: approval-mozilla-beta?

Comment on attachment 9034041 [details]
Bug 1514040 - Dispatch events to hidden datetimebox UA Widget

[Triage Comment]
Fixes a Shadow DOM regression in Fx65. Approved for 65.0b10.

Attachment #9034041 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.