jenkins-bot has submitted this change and it was merged.
Change subject: mw.Target: Fix document focus scroll bug
......................................................................
mw.Target: Fix document focus scroll bug
Follows-up I55ef2622c9eacc which activated code introduced in
mw.Target in commits before that one that caused a change in the
execution order.
Hiding of page content (regular wiki page content provided by
original view request) must happen before the surface document is
focussed.
We used to hide the content from mw.ViewPageTarget#setUpSurface,
which is called from #onReady, which focusses the document after
setUpSurface is done.
Most of this code was moved to mw.ViewPageTarget#onSurfaceReady
which is the listener for the surfaceReady event emitted from
If our surface document gets focus while the original wikipage
content container is still there, the view port is forced to
scroll down because our surface is the next element sibling after
the wikipage container in the DOM.
And browsers (apparently Chrome is not affected) naturally retain
scroll position even if the elements above the one you "scrolled to"
disappear.
We can't (and shouldn't) move the hidePageContent call because
that's the responsibility of the Target subclass, so instead
moved the document focus to below the hidePageContent which is
now also part of the responsibility of the Target subclass.
Also:
* Removed target.surfaceOptions reference because that property
does not exist. We never passed a second argument here, and
whatever this was intended for, doesn't exist.
Bug: 58089
Change-Id: I230fbd5401cbd6e3b9450c7f156650409be8ef16
(cherry picked from commit c6b997d0ba691de03a0125929f1f1c635596d376)
---
M modules/ve-mw/init/targets/ve.init.mw.MobileViewTarget.js
M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
M modules/ve-mw/init/ve.init.mw.Target.js
3 files changed, 31 insertions(+), 2 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve-mw/init/targets/ve.init.mw.MobileViewTarget.js
b/modules/ve-mw/init/targets/ve.init.mw.MobileViewTarget.js
index d6904d2..47541b1 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.MobileViewTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.MobileViewTarget.js
@@ -22,6 +22,11 @@
ve.init.mw.Target.call(
this, $el, mw.config.get( 'wgRelevantPageName' ),
currentUri.query.oldid
);
+
+ // Events
+ this.connect( this, {
+ 'surfaceReady': 'onSurfaceReady'
+ } );
};
/* Inheritance */
@@ -64,3 +69,12 @@
'heading6',
'preformatted'
];
+
+/* Methods */
+
+/**
+ * Once surface is ready ready, init UI.
+ */
+ve.init.mw.MobileViewTarget.prototype.onSurfaceReady = function () {
+ this.$document[0].focus();
+};
diff --git a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
index d9e5495..3287b96 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
@@ -260,11 +260,15 @@
'history': 'updateToolbarSaveButtonState'
} );
this.surface.setPasteRules( this.constructor.static.pasteRules );
+
// Update UI
this.transformPageTitle();
this.changeDocumentTitle();
this.hidePageContent();
this.hideSpinner();
+
+ this.$document[0].focus();
+
this.setupToolbarButtons();
this.attachToolbarButtons();
this.restoreScrollPosition();
diff --git a/modules/ve-mw/init/ve.init.mw.Target.js
b/modules/ve-mw/init/ve.init.mw.Target.js
index eae0218..0eca95f 100644
--- a/modules/ve-mw/init/ve.init.mw.Target.js
+++ b/modules/ve-mw/init/ve.init.mw.Target.js
@@ -37,6 +37,11 @@
this.submitUrl = ( new mw.Uri( mw.util.getUrl( this.pageName ) ) )
.extend( { 'action': 'submit' } );
+ /**
+ * @property {ve.ui.Surface}
+ */
+ this.surface = null;
+
this.modules = [
'ext.visualEditor.core',
'ext.visualEditor.data'
@@ -68,6 +73,12 @@
};
/**
+ * Fired when the #surface is ready.
+ *
+ * By default the surface document is not focussed. If the target wants
+ * the browsers' focus to be in the surface (ready for typing and cursoring)
+ * call `this.$document[0].focus();` in a handler for this event.
+ *
* @event surfaceReady
*/
@@ -310,7 +321,6 @@
this.edited = false;
this.setUpSurface( this.doc, ve.bind( function() {
this.startSanityCheck();
- this.$document[0].focus();
this.emit( 'surfaceReady' );
}, this ) );
};
@@ -1070,7 +1080,7 @@
var dmDoc = ve.dm.converter.getModelFromDom( doc );
setTimeout( function () {
// Create ui.Surface (also creates ce.Surface and
dm.Surface and builds CE tree)
- target.surface = new ve.ui.Surface( dmDoc,
target.surfaceOptions );
+ target.surface = new ve.ui.Surface( dmDoc );
target.surface.$element.addClass(
've-init-mw-viewPageTarget-surface' );
setTimeout( function () {
// Initialize surface
@@ -1078,6 +1088,7 @@
target.$document =
target.surface.$element.find( '.ve-ce-documentNode' );
target.$element.append( target.surface.$element
);
target.setUpToolbar();
+
target.$document.attr( {
'lang': mw.config.get( 'wgVisualEditor'
).pageLanguageCode,
'dir': mw.config.get( 'wgVisualEditor'
).pageLanguageDir
--
To view, visit https://gerrit.wikimedia.org/r/99785
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I230fbd5401cbd6e3b9450c7f156650409be8ef16
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: wmf/1.23wmf6
Gerrit-Owner: Jforrester <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
