Page MenuHomePhabricator

Don't rely on unreliable DOMNodeInsertedIntoDocument event for autosizing TextInputWidget
Closed, ResolvedPublic8 Estimated Story Points

Description

Neither the DOMNodeInsertedIntoDocument event, which we're using for autosizing TextInputWidget, not its non-deprecated replacement, MutationObserver, is supported in all browsers that OOUI targets. This is currently a problem in Firefox and IE 9 and 10. (You can read all about it on T64174 and its abandoned patch.)

We could try polyfilling these, but that would require setting up a polling function that would check the parents chain of the nodes every N milliseconds – probably not something we'd want to do? We could also try hacking up jQuery's function to trigger some events when things get attached, but that's probably even worse.

Probably the only sensible thing we can do is just ask the library's user to call some function after the node is attached…

Event Timeline

matmarex raised the priority of this task from to Medium.
matmarex updated the task description. (Show Details)
matmarex added a project: OOUI.

emit( 'attach' ) seems pretty sensible. We can even have it propagate using either jQuery or the EventEmitter on OO.ui.Element objects.

I'm not sure if it's better to say we should use it everywhere or only in classes that need it. At least emitting it doesn't hurt if there's no one listening, so generic handlers may simply always emit it.

Do we need emit( 'detach' ) as well?

But the problem is, how do we detect nodes being attached/detached in browsers that support neither DOMNodeInsertedIntoDocument nor MutationObserver? Are you saying that jQuery, or our hack into jQuery, should do this?

There were some unclear performance concerns about tracking these events across the entire DOM raised on the abandoned patch.

I've come to the conclusion that detecting attach/detach IE9/10 is pretty much impossible.

However, the event we're really interested in is typically not 'attach' but 'resize'.

There are some solutions out there than can trigger 'resize' events using hidden divs with scroll events attached:

There are some solutions out there than can trigger 'resize' events using hidden divs with scroll events attached:

I tested and these do not detect resizes caused by reattaching an element sized in em to a parent with different font-size.

I chatted about this with Moriel and had a sudden epiphany. Looks like we can support everything including Firefox and IE 9 and 10.

Let's talk Firefox first. The problem with MutationObserver was that it only allows detecting DOM changes inside a specified node, and we want to detect changes outside a node – we want to detect the parent node changing. Thus we'd have to hook it to document root, which was tried with https://gerrit.wikimedia.org/r/#/c/176681/ and rejected on the grounds of unknown performance impact.

But! Eureka! What if we attached the node we're interested in to a "fake" parent we created, not attached anywhere, and instead listened for our node to be removed from this parent? We can then check if a new parent is present, and if not, create and observe another fake. Turns out that this works reliably! (And the performance penalty only applies to this single node we're observing.)

This doesn't help poor IE 9 and 10. But looking at the deprecated mutation events again, there is a DOMNodeRemoved event which can be used to implement the same thing as described above, which they're supposed to support. I have not done it yet, and I don't know about the performance, but it should be possible. If we want to work with even older browsers, continuous polling or explicitly making this library user's responsibility is our only option.

Browser support matrix (per http://caniuse.com/#search=mutation and http://help.dottoro.com/ljfvvdnm.php#additionalEvents):

DOMNodeInsertedIntoDocument (best, but deprecated)MutationObserver (meh, not deprecated)DOMNodeRemoved (meh and deprecated)
ChromeYesYesYes
FirefoxNoYesYes
Opera 12YesNoYes
IE 9-10NoNoYes
IE 11NoYesYes

Change 194197 had a related patch set uploaded (by Bartosz Dziewoński):
TextInputWidget: Use MutationObserver for #onElementAttach support

https://gerrit.wikimedia.org/r/194197

Change 194571 had a related patch set uploaded (by Bartosz Dziewoński):
TextInputWidget: Adjust size and label on first focus, too

https://gerrit.wikimedia.org/r/194571

Change 194197 merged by jenkins-bot:
TextInputWidget: Use MutationObserver for #onElementAttach support

https://gerrit.wikimedia.org/r/194197

Change 194571 merged by jenkins-bot:
TextInputWidget: Adjust size and label on first focus, too

https://gerrit.wikimedia.org/r/194571

What you if you attach your element to another un-attached parent, e.g. attaching an option to a select widget. That would trigger the remove event, but the event you want is when the new parent select widget is attached to the real DOM.

Yes, I realized that this is a problem after commenting here, and solved it in the patch – rather than merely "check if a new parent is present" (=if the element is attached), the code checks if the element's subtree is attached to the document. If it isn't, it starts watching the root of the subtree instead. (This is explained better in code comments.) This is not a complete solution, but it works in most cases (in addition to what you said, I was mostly thinking about Widgets inside FieldLayouts).