Catrope has submitted this change and it was merged. Change subject: Don't trigger namespaced events, it breaks VisualEditor ......................................................................
Don't trigger namespaced events, it breaks VisualEditor Code like $input.trigger( 'focus.ime' ); doesn't make any sense. Event namespaces exist for binding and unbinding and are irrelevant when triggering. However, there's a bug in the version of jQuery that we use (1.8.3) that causes .trigger( 'focus.ime' ) to perform a native focus and call natively bound focus handlers, but only call jQuery event handlers bound to 'focus.ime', not handlers bound to 'focus'. This bug is fixed in jQuery 1.9+. http://jsfiddle.net/WGy9h/3/ demonstrates this bug. The way ULS broke VE with this went like this: * VE initializes and creates two contentEditable divs, the pasteTarget and the documentNode. It then focuses the documentNode. * ULS loads jQuery.ime * Once jQuery.ime loads, it first calls .trigger( 'focus.ime' ) on the pasteTarget * jQuery focuses the pasteTarget, which means a blur is emitted on the documentNode * In response to this blur event, VE disables the SurfaceObserver * jQuery.ime then calls .trigger( 'focus.ime' ) on the documentNode * jQuery focuses the documentNode, but does not call VE's focus handler because it's bound to 'focus' rather than 'focus.ime' * This means VE's SurfaceObserver is not reenabled * If the user then focuses something else then focuses VE again, a native focus event fires and the SurfaceObserver does get reenabled Change-Id: I7c590599df4cf62418403bc1d1dccfc3c6db5fd3 (cherry picked from commit d6ae72eb9c8293f9e814edbe57b10e649ac7c035) --- M resources/js/ext.uls.ime.js 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Catrope: Verified; Looks good to me, approved diff --git a/resources/js/ext.uls.ime.js b/resources/js/ext.uls.ime.js index a3c03a2..a13c770 100644 --- a/resources/js/ext.uls.ime.js +++ b/resources/js/ext.uls.ime.js @@ -205,7 +205,7 @@ if ( !$.ime ) { mw.loader.using( 'jquery.ime', function () { - $input.trigger( 'focus.ime' ); + $input.trigger( 'focus' ); } ); return; @@ -220,7 +220,7 @@ if ( $input.is( '[contenteditable]' ) && !window.rangy ) { // for supporting content editable divs we need rangy library mw.loader.using( 'rangy.core', function () { - $input.trigger( 'focus.ime' ); + $input.trigger( 'focus' ); } ); return; -- To view, visit https://gerrit.wikimedia.org/r/86681 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7c590599df4cf62418403bc1d1dccfc3c6db5fd3 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/UniversalLanguageSelector Gerrit-Branch: wmf/1.22wmf19 Gerrit-Owner: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
