1514040 - AngularJS date input not showing initial value inside directives in Firefox

Status: RESOLVED FIXED

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

Description

[ryanmk54]

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

[Hani Yacoub, Desktop QA]

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.

Comment 2

[Jonathan Watt [:jwatt]]

Given how widely used Angular is, this seems like a P2.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: e9ac4160d051f7ec520c7a0534b75c4b10386873
INFO: First bad revision: 4ec5f59ab7236bce89f734ccdea1ec4c7f5fd95d
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e9ac4160d051f7ec520c7a0534b75c4b10386873&tochange=4ec5f59ab7236bce89f734ccdea1ec4c7f5fd95d

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

:-'(

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1514040 - Dispatch events to hidden datetimebox UA Widget

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.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(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 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6dd228164d42
Dispatch events to hidden datetimebox UA Widget r=smaug

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/6dd228164d42

Comment 12

[Emilio Cobos Álvarez (:emilio)]

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?

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

We are dispatching the events to Shadow DOM. I don't think UA Widget can hear the event if you pass yes there.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Need uplift

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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:

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/af69c79ed942