Last modified: 2014-09-17 21:25:25 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T67608, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 65608 - OOjs UI: onDomEvent('focus') does not work after jQuery upgrade
OOjs UI: onDomEvent('focus') does not work after jQuery upgrade
Status: RESOLVED INVALID
Product: OOjs UI
Classification: Unclassified
General (Other open bugs)
unspecified
All All
: High major
: ---
Assigned To: Krinkle
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-21 21:52 UTC by Tisza Gergő
Modified: 2014-09-17 21:25 UTC (History)
1 user (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Tisza Gergő 2014-05-21 21:52:06 UTC
Steps to reproduce:
1. create OOJS UI textbox widget
2. bind onDomEvent focus handler
3. click on it, and observe the nothingness

jQuery 1.9 has a nasty change in how triggered focus events are handled. There is some documentation on it at [1] which states "jQuery has always ensured that a call to .trigger("focus") or .focus() consistently runs any attached event handlers, even if the element cannot be focused, and jQuery 1.9 continues to do that."

This is a bald-faced lie. They actually just call the element's native focus() method and hope for the best. The related code is in $.trigger [2] which calls the special focus triggering logic [3] which will return false and cancel the simulated event bubbling of jQuery.

This results in all kind of fun behavior changes, see e.g. [4] Specifically in the case of OOJS UI textboxes, it seems that setting these events on the parent element of the textarea instead of the textarea itself prevents the event triggering somehow.

For a live example, check [5]. This is the textbox in MediaViewer's "Use this file" menu which should select the text when you click in it. I've verified that this works with the current core and MediaViewer master, but resources/lib/jquery/jquery.js rolled back to the old version, but does not work with the new jQuery version. (Also verified that commenting out line 107 does not fix it, so it is not our manual focus triggering that causes this.) This commit [6] fixed the issue.



[1] http://jquery.com/upgrade-guide/1.9/#order-of-triggered-quot-focus-quot-events
[2] https://github.com/jquery/jquery/blob/1.11.0/src/event.js#L281
[3] https://github.com/jquery/jquery/blob/1.11.0/src/event.js#L576
[4] http://bugs.jquery.com/ticket/13363
[5] https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FMultimediaViewer/f71a591189bb76abb0a178183617be4f33ebacd0/resources%2Fmmv%2Fui%2Fmmv.ui.reuse.share.js#L107
[6] https://gerrit.wikimedia.org/r/134738
Comment 1 Krinkle 2014-06-26 23:48:32 UTC
(In reply to Tisza Gergő from comment #0)
> jQuery 1.9 has a nasty change in how triggered focus events are handled.
> There is some documentation on it at [1] which states "jQuery has always
> ensured that a call to .trigger("focus") or .focus() consistently runs any
> attached event handlers, even if the element cannot be focused, and jQuery
> 1.9 continues to do that."
> 
> This is a bald-faced lie. 

No. This is taken out of context. Full two paragraph froms
http://jquery.com/upgrade-guide/1.9/#order-of-triggered-quot-focus-quot-events

"When the user clicks or tabs into a form element to bring it into focus, the browser first fires a blur event for the previously focused element and then a focus event for the new element. Prior to 1.9, a trigger()ed focus event using either .trigger("focus") or .focus() would fire a focus event for the new element and then the blur event for the previous element before finally actually focusing the element. In 1.9 this behavior has been changed to reflect the same order as if the user had caused the focus change."

"With native DOM focus events, the browser only calls a focus event handler if the target element is not already focused and can also successfully be focused. jQuery has always ensured that a call to .trigger("focus") or .focus() consistently runs any attached event handlers, even if the element cannot be focused, and jQuery 1.9 continues to do that. This is different behavior than the DOM .focus() method, which will not call event handlers in many cases including where the element is already focused or the element is disabled."

The first paragraph explains the fixed behaviour where it no longer manually fires jQuery-bound event handlers first, and only trigger native focus() after – which then triggers blur first – thus causing the blur to effectively happen after the focus handlers are fired (blur bubbles back into jQuery and triggers those).

As a result of using the native handlers and triggering them in the right order, elements that are never focusable to begin with are no longer getting any handlers triggers. Previously they were getting a partial/incomplete event sequence (only the jQuery-bound handlers, but no blur or native focus of course).

While the second paragraph is somewhat ambiguous, I don't think the sentence you cited is saying it will continue to run on all elements (including those not focusable in general). As emphasised by the sentence that follows that one, it is saying they will run the handlers even if the element is already focussed (which is also an instance of "cannot be focussed", albeit temporarily).
Comment 2 James Forrester 2014-07-16 01:34:48 UTC
Gergő, can we close this as INVALID?
Comment 3 Tisza Gergő 2014-07-16 06:18:52 UTC
Textual criticism of the jQuery release announcement aside, the OOjs UI textinput does not trigger its focus handler when focused. I don't think that's expected behavior.

Here is a minimal testcase: http://jsfiddle.net/79gHj/
Compare with the click event, which works as expected: http://jsfiddle.net/3CHT4/
Comment 4 Krinkle 2014-08-27 13:18:46 UTC
OO.ui.Element#onDomEvent is deprecated because it's just a legacy alias for jQuery#on. I don't think this is an OOjs UI bug.

It may or may not have worked before, and that use case may or may not have been documented/supported by jQuery. Either way, I don't think this is something we should tackle in OOjs UI.

I'd be interested to see if you've come up with a work-around and what that looks like. Do you know of other libraries/applications that have faced a similar issue on the web? Is there an upstream bug report?
Comment 5 Tisza Gergő 2014-09-17 21:25:25 UTC
Sorry for the slow answer. As mentioned in comment 0, our workaround was https://gerrit.wikimedia.org/r/134738 - set the event handler manually instead of using onDomEvent. The broken behavior was related specifically to OOUI listening for the focus event on the div wrapping the textbox, and not the textbox itself. So we listened manually on the textbox itself, and that worked fine. Could be some sort of Firefox bug where the event did not bubble; I did not investigate. 

I did not search for bug reports from other applications; if by upstream you mean jQuery, they got several bug reports about this (one is linked in comment 0), but argued that this is still the better tradeoff, as opposed to simulating the focus event and messing up the sequence in which events happen.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links