Page MenuHomePhabricator
Create Task
Maniphest T188077

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

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

Details

SubjectRepoBranchLines +/-
Fix: jQuery.fn.offset() requires an element connected to a documentmediawiki/extensions/ContentTranslationmaster+15 -4
Customize query in gerrit

Related Objects

Event Timeline

Nikerabbit created this task.Feb 23 2018, 8:59 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 23 2018, 8:59 AM
Pginer-WMF triaged this task as Medium priority.Feb 23 2018, 10:21 AM
Pginer-WMF raised the priority of this task from Medium 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 Medium priority.Feb 23 2018, 10:25 AM
Pginer-WMF moved this task from Needs Triage to CX2 on the ContentTranslation board.Mar 1 2018, 4:45 PM
Nikerabbit claimed this task.Mar 26 2018, 1:56 PM
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.

Pginer-WMF added a project: Language-2018-Apr-June.Mar 28 2018, 9:59 AM
Nikerabbit moved this task from QA to In Progress on the Language-2018-Jan-Mar board.Mar 29 2018, 6:37 AM

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
Pginer-WMF moved this task from Backlog to In Progress on the Language-2018-Apr-June board.Apr 3 2018, 8:13 AM
Pginer-WMF removed a project: Language-2018-Jan-Mar.Apr 3 2018, 8:26 AM
Nikerabbit moved this task from In Progress to Maintenance backlog on the Language-2018-Apr-June board.Apr 5 2018, 7:38 AM
• santhosh edited projects, added Language-2018-Jan-Mar, VisualEditor; removed Language-2018-Apr-June.Apr 9 2018, 10:37 AM
• santhosh subscribed.

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();
Nikerabbit added a comment.Apr 9 2018, 1:29 PM

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.

Nikerabbit added a comment.Apr 9 2018, 2:15 PM

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.

gerritbot subscribed.Apr 9 2018, 2:19 PM

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

gerritbot added a project: Patch-For-Review.Apr 9 2018, 2:19 PM
Nikerabbit moved this task from In Progress to In Review on the Language-2018-Jan-Mar board.Apr 9 2018, 2:37 PM
Nikerabbit added a project: Language-2018-Apr-June.Apr 9 2018, 2:39 PM
Nikerabbit moved this task from Maintenance backlog to In Review on the Language-2018-Apr-June board.
gerritbot added a comment.Apr 10 2018, 10:00 AM

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-10 (1.31.0-wmf.29)).Apr 10 2018, 10:00 AM
Nikerabbit moved this task from In Review to QA on the Language-2018-Apr-June board.Apr 10 2018, 10:18 AM
Nikerabbit removed a project: Patch-For-Review.
KartikMistry subscribed.Apr 25 2018, 5:54 AM

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
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL