1467586 - Only load the <input type="time/date"> picker code when it's used

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement)

Description

[:Felipe Gomes (needinfo for replies!)]

This code is loaded unconditionally in browser-content.js, but it could be instantiated only when used. Not only that, this feature is actually disabled by default, so we should skip loading it unless it's toggled on.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1467586 - Part 0. hg cp the DateTimePickerListener object to a new file, and rename the DateTimePickerHelper.

Since we're naming this new file as DateTimePickerContent, it's also a good opportunity to rename the DateTimePickerHelper to DateTimePickerParent to make it more meaningful

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

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1467586 - Convert the DateTimePickerListener object into a class to be instantiated when needed.

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

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8984288 [details]
Bug 1467586 - Part 0. hg cp the DateTimePickerListener object to a new file, and rename the DateTimePickerHelper.

https://reviewboard.mozilla.org/r/250096/#review256776

Thanks!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8984289 [details]
Bug 1467586 - Convert the DateTimePickerListener object into a class to be instantiated when needed.

https://reviewboard.mozilla.org/r/250098/#review256778

This is great! Assuming this passes the datetime picker mochitests, this looks good to go from me. Good work!

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8984288 [details]
Bug 1467586 - Part 0. hg cp the DateTimePickerListener object to a new file, and rename the DateTimePickerHelper.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250096/diff/1-2/

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8984289 [details]
Bug 1467586 - Convert the DateTimePickerListener object into a class to be instantiated when needed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250098/diff/1-2/

Comment 7

[:Felipe Gomes (needinfo for replies!)]

I made a small change because I misunderstood the meaning of the pref. The pref only blocks the code for input type="time", but this code is also used for input type="date". So I moved the pref check back to its original location, and then just declared the object as a lazy proxy.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f8e35e87a21
Part 0. hg cp the DateTimePickerListener object to a new file, and rename the DateTimePickerHelper. r=mconley
https://hg.mozilla.org/integration/autoland/rev/79476ed91dec
Convert the DateTimePickerListener object into a class to be instantiated when needed. r=mconley

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5f8e35e87a21
https://hg.mozilla.org/mozilla-central/rev/79476ed91dec