Page MenuHomePhabricator

CX2 translation view: JQMIGRATE: jQuery.fn.offset() requires an element connected to a document
Closed, ResolvedPublic

Description

New warning in the JS console. This looks like something that will break when JQMIGRATE is taken away. Introduced in https://gerrit.wikimedia.org/g/mediawiki/extensions/ContentTranslation/+/74da638516df477d764fa95d4ed968fd147cd77e

JQMIGRATE: jQuery.fn.offset() requires an element connected to a document
migrateWarn	@	load.php?debug=false…version=1bu6j6a:140
jQuery.fn.offset	@	load.php?debug=false…version=1bu6j6a:148
ve.ui.PositionedTargetToolbar.calculateOffset	@	load.php?debug=false…version=0j5i3ga:577
ve.ui.PositionedTargetToolbar.getElementOffset	@	load.php?debug=false…version=0j5i3ga:577
ve.init.Target.onContainerScroll	@	VM220:216
requestAnimationFrame (async)		
ve.init.Target.setupToolbar	@	VM220:219
ve.init.mw.CXTarget.setupToolbar	@	VM220:1207
ve.init.Target.setSurface	@	VM220:218
ve.init.mw.Target.setSurface	@	VM220:339
ve.init.mw.CXTarget.setTranslation	@	VM220:1209

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 23 2018, 8:59 AM
Pginer-WMF triaged this task as Normal priority.Feb 23 2018, 10:21 AM
Pginer-WMF raised the priority of this task from Normal to Needs Triage.
Pginer-WMF moved this task from Backlog to Maintenance backlog on the Language-2018-Jan-Mar board.
Pginer-WMF triaged this task as Normal priority.Feb 23 2018, 10:25 AM
Nikerabbit moved this task from Maintenance backlog to QA on the Language-2018-Jan-Mar board.

Looks like either a change in VE or CX has fixed this. Not surprising given the changes to the toolbar code.

Seeing this again, but from a different source now, related to alignSectionPairs.

ve.init.mw.CXTarget.js?61c8f:317 JQMIGRATE: jQuery.fn.offset() requires an element connected to a document
migrateWarn	@	load.php?debug=true&…rsion=1gsvt61:10340
jQuery.fn.offset	@	load.php?debug=true&…rsion=1gsvt61:10787
ve.init.mw.CXTarget.alignSectionPairs	@	ve.init.mw.CXTarget.js?61c8f:317
run	@	core.js:281
(anonymous)	@	core.js:298
ve.init.mw.CXTarget.onTargetTitleChange	@	ve.init.mw.CXTarget.js?61c8f:201
oo.EventEmitter.emit	@	oojs.jquery.js?4bc88:829
oo.EventEmitter.emit	@	oojs.jquery.js?4bc88:829
OO.ui.InputWidget.setValue	@	InputWidget.js:182
mw.cx.ui.TargetColumn.setTargetTitle	@	mw.cx.ui.TargetColumn.js?e5e1c:79
mw.cx.ui.TargetColumn.setTranslation	@	mw.cx.ui.TargetColumn.js?e5e1c:75
ve.init.mw.CXTarget.setTranslation	@	ve.init.mw.CXTarget.js?61c8f:112
(anonymous)	@	mw.cx.init.Translation.js?35839:87
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3583
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3823
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3823
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3642
process	@	load.php?debug=true&…ersion=1gsvt61:3651
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3681
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3583
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3642
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
deferred.(anonymous function)	@	load.php?debug=true&…ersion=1gsvt61:3778
(anonymous)	@	api.js?8308a:274
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
done	@	load.php?debug=true&…ersion=1gsvt61:9272
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:9514
load (async)		
send	@	load.php?debug=true&…ersion=1gsvt61:9533
ajax	@	load.php?debug=true&…ersion=1gsvt61:9173
jQuery.ajax	@	load.php?debug=true&…rsion=1gsvt61:10483
ajax	@	api.js?8308a:246
get	@	api.js?8308a:134
mw.cx.init.Translation.fetchDraft	@	mw.cx.init.Translation.js?35839:308
(anonymous)	@	mw.cx.init.Translation.js?35839:130
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3583
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3642
process	@	load.php?debug=true&…ersion=1gsvt61:3651
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3681
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3583
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
add	@	load.php?debug=true&…ersion=1gsvt61:3376
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3709
jQuery.Deferred	@	load.php?debug=true&…rsion=1gsvt61:10866
then	@	load.php?debug=true&…ersion=1gsvt61:3694
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3621
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3642
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
fire	@	load.php?debug=true&…ersion=1gsvt61:3455
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
deferred.(anonymous function)	@	load.php?debug=true&…ersion=1gsvt61:3778
(anonymous)	@	api.js?8308a:274
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
fireWith	@	load.php?debug=true&…ersion=1gsvt61:3447
done	@	load.php?debug=true&…ersion=1gsvt61:9272
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:9514
load (async)		
send	@	load.php?debug=true&…ersion=1gsvt61:9533
ajax	@	load.php?debug=true&…ersion=1gsvt61:9173
jQuery.ajax	@	load.php?debug=true&…rsion=1gsvt61:10483
ajax	@	api.js?8308a:246
get	@	api.js?8308a:134
mw.cx.init.Translation.fetchDraftInformation	@	mw.cx.init.Translation.js?35839:244
mw.cx.init.Translation.fetchTranslationData	@	mw.cx.init.Translation.js?35839:123
mw.cx.init.Translation.init	@	mw.cx.init.Translation.js?35839:66
initCX	@	mw.cx.init.js?efea0:42
mightThrow	@	load.php?debug=true&…ersion=1gsvt61:3583
process	@	load.php?debug=true&…ersion=1gsvt61:3651
setTimeout (async)		
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3689
fire	@	load.php?debug=true&…ersion=1gsvt61:3317
add	@	load.php?debug=true&…ersion=1gsvt61:3376
(anonymous)	@	load.php?debug=true&…ersion=1gsvt61:3709
jQuery.Deferred	@	load.php?debug=true&…rsion=1gsvt61:10866
then	@	load.php?debug=true&…ersion=1gsvt61:3694
jQuery.fn.ready	@	load.php?debug=true&…ersion=1gsvt61:3882
jQuery.fn.init	@	load.php?debug=true&…ersion=1gsvt61:3050
jQuery.fn.init	@	load.php?debug=true&…rsion=1gsvt61:10393
jQuery	@	load.php?debug=true&…&version=1gsvt61:98
(anonymous)	@	mw.cx.init.js?efea0:54
(anonymous)	@	mw.cx.init.js?efea0:55
santhosh added a subscriber: santhosh.

T177252: JQMIGRATE warning "jQuery.fn.offset() requires an element connected to a document" when the element is the root had similar problem and see the discussion.

Following patch can replace the offset() call with native getBoundingClientRect and avoid the warning.

diff --git a/modules/ve-cx/init/ve.init.mw.CXTarget.js b/modules/ve-cx/init/ve.init.mw.CXTarget.js
index aee2a092..0d5a8662 100644
--- a/modules/ve-cx/init/ve.init.mw.CXTarget.js
+++ b/modules/ve-cx/init/ve.init.mw.CXTarget.js
@@ -312,11 +312,13 @@ ve.init.mw.CXTarget.prototype.onDocumentTransact = function () {

 ve.init.mw.CXTarget.prototype.alignSectionPairs = function () {
        var i, sourceOffsetTop, documentNodeChildren, targetOffsetTop, alignSectionPair,
-               sourceDocumentNode, articleNode;
+               sourceDocumentNode, targetDocumentNode, articleNode,
+               scrollTop = document.body.scrollTop;

        sourceDocumentNode = this.sourceSurface.getView().getDocument().getDocumentNode();
-       sourceOffsetTop = sourceDocumentNode.$element.offset().top;
-       targetOffsetTop = this.targetSurface.getView().getDocument().documentNode.$element.offset().top;
+       sourceOffsetTop = sourceDocumentNode.$element[ 0 ].getBoundingClientRect().top + scrollTop;
+       targetDocumentNode = this.targetSurface.getView().getDocument().getDocumentNode();
+       targetOffsetTop = targetDocumentNode.$element[ 0 ].getBoundingClientRect().top + scrollTop;
        alignSectionPair = this.translationView.constructor.static.alignSectionPair;
        documentNodeChildren = sourceDocumentNode.getChildren();

In this case I doubt it is about the htmlelement (and if it were I believe the fix would be different). It is nice that JQMIGRATE gives the warning, because for unattached elements we just get zeroes which is not what we want either:

$( '<div>' )[0].getBoundingClientRect()
{x: 0, y: 0, width: 0, height: 0, top: 0, …}

Could there be a race condition where alignSectionPairs is called before restoration is complete? There are many callers of this method so it is hard to check, and I cannot reliably reproduce it.

Actually I can, I just need to change the target title of the page from the default one to trigger this. I'm proposing an alternative fix.

Change 425058 had a related patch set uploaded (by Nikerabbit; owner: Nikerabbit):
[mediawiki/extensions/ContentTranslation@master] Fix: jQuery.fn.offset() requires an element connected to a document

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

Change 425058 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@master] Fix: jQuery.fn.offset() requires an element connected to a document

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

Nikerabbit removed a project: Patch-For-Review.

Fixed in CX2 and deployed in Production now.

KartikMistry moved this task from QA to Done on the Language-2018-Apr-June board.Apr 25 2018, 5:55 AM
Pginer-WMF closed this task as Resolved.Apr 30 2018, 8:00 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptApr 30 2018, 8:00 AM