Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/232893
Change subject: Make focusing surfaces work in Firefox
......................................................................
Make focusing surfaces work in Firefox
When you called .focus() on a blurred surface, the toolbar
would be enabled, but the cursor wouldn't actually go into
the surface, and keystrokes wouldn't be picked up.
This happened because the .focus() code path for NullSelection
relied on showSelectionState() to do a DOM .focus() call.
However, showSelectionState()'s check for whether it should focus
a DOM node was broken: when nothing is selected,
document.activeElement === document.body, so
OO.ui.contains( document.activeElement, newSel.focusNode ) is true
(because everything is inside the <body>), so a DOM focus
never happened.
Instead of checking whether newSel.focusNode is inside the currently
focused element (which has all sorts of other problems aside from
the default being the <body>), determine which node we would focus
if we needed to, then check if activeElement is inside it.
Additionally, make .focus() not rely on showSelectionState()'s focusing
behavior, but instead have it normalize NullSelections to the first
content offset, and then proceed to focus the surface. This will
cause a double focus so long as showSelectionState() behaves the
way it currently does, and it'll invoke onModelSelect() twice,
but that shouldn't cause any problems. In particular, showSelectionState()
short-circuits when the selection is already in the desired state.
Bug: T109805
Change-Id: I33fefbccd5c4e5abae77b01d14238257e1360799
---
M src/ce/ve.ce.Surface.js
1 file changed, 9 insertions(+), 5 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/93/232893/1
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 910e485..80a003e 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -611,15 +611,17 @@
surface = this,
selection = this.getModel().getSelection();
+ if ( selection instanceof ve.dm.NullSelection ) {
+ this.getModel().selectFirstContentOffset();
+ selection = this.getModel().getSelection();
+ }
+
// Focus the documentNode for text selections, or the pasteTarget for
focusedNode selections
if ( this.focusedNode || selection instanceof ve.dm.TableSelection ) {
this.$pasteTarget[ 0 ].focus();
} else if ( selection instanceof ve.dm.LinearSelection ) {
node = this.getDocument().getNodeAndOffset(
selection.getRange().start ).node;
$( node ).closest( '[contenteditable=true]' )[ 0 ].focus();
- } else if ( selection instanceof ve.dm.NullSelection ) {
- this.getModel().selectFirstContentOffset();
- return;
}
// If we are calling focus after replacing a node the selection may be
gone
@@ -3847,6 +3849,7 @@
*/
ve.ce.Surface.prototype.showSelectionState = function ( selection ) {
var range,
+ $focusTarget,
extendedBackwards = false,
sel = this.nativeSelection,
newSel = selection;
@@ -3887,8 +3890,9 @@
// Setting a range doesn't give focus in all browsers so make sure this
happens
// Also set focus after range to prevent scrolling to top
- if ( !OO.ui.contains( this.getElementDocument().activeElement,
newSel.focusNode, true ) ) {
- $( newSel.focusNode ).closest( '[contenteditable=true]'
).focus();
+ $focusTarget = $( newSel.focusNode ).closest( '[contenteditable=true]'
);
+ if ( !OO.ui.contains( $focusTarget.get( 0 ),
this.getElementDocument().activeElement, true ) ) {
+ $focusTarget.focus();
} else {
// Scroll the node into view
ve.scrollIntoView(
--
To view, visit https://gerrit.wikimedia.org/r/232893
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I33fefbccd5c4e5abae77b01d14238257e1360799
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits