Page MenuHomePhabricator

VisualEditor "jumps down" after editor loads in Safari
Closed, ResolvedPublic

Description

In Safari, when the editor opens, the viewport jumps down past the site navigation header. This does not happen in Firefox or Chrome. It might be biased by having mainly used Firefox as my default browser past year (and Chrome before that), but I found it more natural for it to not scroll down (like we do in Firefox) as it meant the visual context is more preserved. In Chrome and Safari, it felt odd for the content to "jump down". Especially since this curently only happens after the end of the loading bar is reached.

SafariFirefox
Screenshot 2020-07-24 at 23.34.21.png (1×2 px, 1011 KB)
Screenshot 2020-07-24 at 23.35.39.png (1×2 px, 984 KB)

Event Timeline

Krinkle renamed this task from VisualEditor "jumps down" when loading editor in Safari to VisualEditor "jumps down" after editor loads in Safari.Jul 24 2020, 11:05 PM
JTannerWMF added a subscriber: JTannerWMF.

We will take a look at this at some point this quarter

matmarex added a subscriber: matmarex.

It seems to be scrolling so that your text cursor is at the top of the window. No idea whether that's our fault or Safari's.

I was looking at this -- it only happened to me while logged out. As soon as I logged in, the behavior went back to being consistent.

I agree with @Krinkle in thinking it's important for people to be able to "orient themselves" upon arriving in "edit mode" from "read mode" and I can see how the viewport scrolling on its own could disrupt that feeling.

I was looking at this -- it only happened to me while logged out. As soon as I logged in, the behavior went back to being consistent.

With the above said, I have not been able to reproduce this while logged in or out in Chrome: Version 89.0.4389.72 (Official Build) (x86_64).

@ppelberg Aye, but the issue is in Safari, not Chrome! I can still reproduce it today.

@ppelberg Aye, but the issue is in Safari, not Chrome! I can still reproduce it today.

Yikes 🤦‍♂️– you're right. I am able to reproduce this:

Safari Version 14.0.3 (16610.4.3.1.4)
https://youtu.be/oEORWTfiErE

I'll confirm this is still happening in Safari 15, though inconsistently. When logged out, the initial showing of the welcome-dialog seemed to prevent the jump, so it only happened on my second load of the article.

image.png (2×5 px, 3 MB)

This is caused by Safari deciding it needs to scroll the surface into view when it is focused. We transfer focus to the surface if it isn't there to begin with, this happens when the edit notice box is open (the close button takes focus).

This happens in ve.ce.Surface.prototype.showSelectionState:

	// 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
	var $focusTarget = $( newSel.focusNode ).closest( '[contenteditable=true]' );
	if ( $focusTarget.get( 0 ) === this.getElementDocument().activeElement ) {
		// Already focused, do nothing.
		// Support: IE<=11
		// Focusing already-focused elements scrolls the *top* of the element
		// into view, meaning that in long text blocks refocusing the current
		// element will jump the viewport around.
		// Check $focusTarget is non-empty (T259531)
	} else if ( $focusTarget.length && !OO.ui.contains( $focusTarget.get( 0 ), this.getElementDocument().activeElement ) ) {
		// Note: contains *doesn't* include === here. This is desired, as the
		// common case for getting here is when pressing backspace when the
		// cursor is in the middle of a block of text (thus both are a <div>),
		// and we don't want to scroll away from the caret.
		$focusTarget.trigger( 'focus' );
	} else {
		// Scroll the node into view
		ve.scrollIntoView(
			$( newSel.focusNode ).closest( '*' ).get( 0 )
		);
	}

Change 740285 had a related patch set uploaded (by Esanders; author: Esanders):

[VisualEditor/VisualEditor@master] Prevent scroll jump when focusing surface in Safari

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

Test wiki created on Patch Demo by ESanders (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/25bcaa7ec4/w/

DLynch added a project: Editing QA.

QA testing note: you need to be on a page which is taller than your current window to see anything happen.

Change 740285 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Prevent scroll jump when focusing surface in Safari

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

Change 740629 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (5a97be3a0)

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

Change 740629 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (5a97be3a0)

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

Thanks for the testing note @DLynch. I tested on a tall enough page.

The links were helpful @Esanders . It helped with recreating this. All good now, thanks.

For visual clarity, check https://youtu.be/oEORWTfiErE (thanks for this @ppelberg ) vs https://photos.app.goo.gl/1BGy5F8ygtgdudf19.