1446342 - Input type="date" not working if the other form elements has name="document"

Status: RESOLVED FIXED

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

Description

[Sean Greenhow]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180206200532

Steps to reproduce:

Using the HTML below i tried to create a form with a date input and a file input.
As long as the file input has the name property set the date input will not work correctly.


My current Firefox version is 58.0.2 (64-Bit)

I also tested it with a fresh installation of Firefox Developer Edition (ver. 60.0b3 64-bit)

My locale is DE_de, maybe that makes a difference due to localization...


This fiddle shows the problem:
https://jsfiddle.net/chyemk4a/3/


This is the HTML Code used:
<form>
   <input type="date">
   <input name="document" type="file">
</form>

 <form>
   <input type="date">
   <input type="file">
</form>


Actual results:

The date input shows "TT.MM.JJJJ" as well as the "X"-Button to clear the input. 
Pressing the "X"-Button does NOT do anything. 
Clicking any of the parts of the date does NOT trigger the date picker to slide down.


Expected results:

The date input should not show the "X"-Button, since the input does not have any value set yet.
The date picker should slide down when any of the parts of the date is clicked.

Comment 1

[Alice0775 White]

the problem is name="document", 
the following html is also not working.

<form>
   <textarea name="document"></textarea>
   <input type="date">
</form>

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(In reply to Alice0775 White from comment #1)
> the problem is name="document", 
> the following html is also not working.
> 
> <form>
>    <textarea name="document"></textarea>
>    <input type="date">
> </form>

Hmm, that looks really fishy, great find.

So the key is that when there's name=document on another input, `document` stops meaning what it's intended to mean in [1]. Indeed, replacing all the uses of document for window.document in that file fixes it for me, but I still don't get why...

Bobby, you know about XBL in content and whatever happens to the scope object in that case. I suspect `document` here starts going through the window named getter incorrectly (using all the global scope polluter stuff)... May that be the case?

[1]: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/datetimebox.xml

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Indeed that's what's happening, in a debug build I can see:

  document.getAnonymousElementByAttribute is not a function

in the logs.

Comment 4

[Boris Zbarsky [:bzbarsky]]

XBL constructors and destructors run with the same scoping setup as attribute event handlers.  See the nsJSUtils::GetScopeChainForElement in nsXBLProtoImplAnonymousMethod::Execute.

For XBL attached to a form control, that means that the scope chain has the control, the form, the document, then the window in that order.

HTMLFormElement has an [OverrideBuiltins] named getter.  So "document" in the constructor ends up found on the form, if there is a control named "document".

The only reason we did the scope chain thing here is that XBL always used to do this and we did not want to break compat.  Given how much less (especially unknown) XBL there is nowadays, we could try changing the scope chain for the constructor/destructor.  We could also add a way for bindings to opt out of the weird scope chain and use it for any binding that gets attached to web content.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I guess this would not only be about constructor / destructors in this case... Let's fix the binding for now. We're looking into getting rid of these anyway.

Comment 6

[Phabricator Automation]

Attachment: Bug 1446342: Fix XBL scope footgun in datetimebox bindings. r=smaug

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960036 [details]
Bug 1446342: Fix XBL scope footgun in datetimebox bindings. r=smaug

I guess someone from the frontend team should review this as well.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8960036 [details]
Bug 1446342: Fix XBL scope footgun in datetimebox bindings. r=smaug

I gave r- to some patch, but it wasn't reflected to bugzilla at all :/

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960036 [details]
Bug 1446342: Fix XBL scope footgun in datetimebox bindings. r=smaug

Fixed those. I can look into why DateTimeFormat is not Xrayable tomorrow.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960036 [details]
Bug 1446342: Fix XBL scope footgun in datetimebox bindings. r=smaug

So, after talking a bit more with Olli, I changed my mind about the right approach. I initially thought that this would still be a footgun for the addEventListener case, but looking through the ELM code turns out it's not.

So it feels way less error prone to just hardcode the event target chain for XBL to be BoundElement -> OwnerDoc() instead.

And XRays should protect us from stuff like document.querySelector("video").document = "garbage", so it looks much nicer.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #10) 
> So it feels way less error prone to just hardcode the event target chain for
> XBL to be BoundElement -> OwnerDoc() instead.
What? We can't do that.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

I'm sure you mean scope, and not event target chain.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

(In reply to Olli Pettay [:smaug] from comment #12)
> I'm sure you mean scope, and not event target chain.

Err, yes, sorry. Brainfarted.

Comment 14

[Phabricator Automation]

Attachment: Bug 1446342: Don't include forms in the scope chain for XBL. r=smaug

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960041 [details]
Bug 1446342: Don't include forms in the scope chain for XBL. r=smaug

Even with tests ;)

Comment 16

[Boris Zbarsky [:bzbarsky]]

> I guess this would not only be about constructor / destructors in this case...

Well, more precisely it's relevant to the following:

1)  XBL constructors/destructors.
2)  XBL field evaluations.
3)  Event listeners compiled from <xbl:handler>

but datetimebox.xml has no fields or handlers.

> I can look into why DateTimeFormat is not Xrayable tomorrow.

Because it never got implemented so far.  JS things have to opt in to Xrays (and get audited in the process) to get Xrayed.  See IsJSXraySupported in js/xpconnect/wrappers/XrayWrapper.cpp

I left some comments on the latest patch in phabricator, but the way the discussion gets fragmented between here and there, and between the multiple revisions there is ... deeply unfortunate.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960041 [details]
Bug 1446342: Don't include forms in the scope chain for XBL. r=smaug

Made it opt-in via an attribute instead. FWIW the only thing that the previous patch broke was the findbar.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8960041 [details]
Bug 1446342: Don't include forms in the scope chain for XBL. r=smaug

Fixed up, also renamed to simpleScopeChain as suggested by Boris.

Comment 19

[Phabricator Automation]

Comment on attachment 8960041 [details]
Bug 1446342: Don't include forms in the scope chain for XBL. r=smaug

Olli Pettay [:smaug] has approved the revision.

https://phabricator.services.mozilla.com/D769

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05270b574f00
Don't include forms in the scope chain for XBL datetime bindings. r=smaug

Comment 21

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/05270b574f00