Page MenuHomePhabricator

Initiating VisualEditor via "Edit" button Firefox scrolls to bottom of page
Closed, ResolvedPublic1 Story Points

Description

  • Open a relatively long article using Firefox 35 (stable).
  • Initiate VisualEditor via the "Edit" tab (it seems veaction=edit is unaffected).

Unexpected:
After the surface is ready the page is scrolled to the far bottom.

When manually scrolling back up it is observed that the cursor is actually at the top in the expected position, but for some reason the page happened to scroll down anyway.

I can consistently reproduce this on:

Event Timeline

Krinkle created this task.Feb 23 2015, 9:25 AM
Krinkle updated the task description. (Show Details)
Krinkle raised the priority of this task from to Needs Triage.
Krinkle added a subscriber: Krinkle.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 23 2015, 9:25 AM

After a bit of debugging, I believe I found the culprit of this issue, but I am not entirely sure what to do to fix it.

The page doesn't just scroll downwards, it seems it scrolls incrementally, right after the call to this.getSurface().getView().focus(); in ve.init.mw.ViewPageTarget.prototype.onSurfaceReady (line 532)
Then when
this.getModel().selectFirstContentOffset(); is called, and in it "this.setLinearSelection( new ve.Range( firstOffset ) );" -- and at that point, scrolls halfway down, seemingly to nodes that are being rendered.
By the time that "this.activatingDeferred.resolve();" is called from onSurfaceReady, the page scrolled all the way down.

I am not sure how to fix this, but it seems to be related to selections and focus in firefox.

This is caused by the line

		$( rangeSelection.start.node ).closest( '[contenteditable=true]' ).focus();

in ve.ce.Surface.prototype#showSelection

In Firefox, we seem to have the page duplicated at this point of the initialization process -- the read page, and underneath it the contentEditable. When the code requests to focus() it does so -- but it does so to the bottom (or "middle" of the current double-page). After that we seem to bring the contentEditable upwards, but at that point the focus is already at the bottom of the page.

In Firefox, we seem to have the page duplicated at this point of the initialization process -- the read page, and underneath it the contentEditable.

This happens in all browsers, but in Chrome it seems like this code path isn't being taken, and doesn't result in scrolling in any case.

When the code requests to focus() it does so -- but it does so to the bottom (or "middle" of the current double-page). After that we seem to bring the contentEditable upwards, but at that point the focus is already at the bottom of the page.

Yeah, the problem is that we're focusing the surface when the original page content is still visible above it. What's the stack trace of that focus call? I.e. what's calling what to cause that focus to occur?

The stack call:

  • ve.init.mw.Target.prototype#onReady:474
    • target.emit( 'surfaceReady' );
  • OO.EventEmitter#emit:711
    • method.apply( ... )
  • ve.init.mw.ViewPageTarget.prototype#onSurfaceReady:532
    • this.getSurface().getView().focus();
  • ve.ce.Surface.prototype#focus:
    • this.getModel().selectFirstContentOffset();
  • ve.dm.Surface.prototype#selectFirstContentOffset:701
    • this.setLinearSelection( new ve.Range( firstOffset ) );
  • ve.dm.Surface.prototype#setLinearSelection:558
    • this.setSelection( new ve.dm.LinearSelection( this.getDocument(), range ) );
  • ve.dm.Surface.prototype#setSelection:684
    • this.emit( 'select', this.selection.clone() );
  • OO.EventEmitter#emit:711
    • method.apply( ... )
  • ve.ce.Surface.prototype#onModelSelect:2193
    • this.showSelection( selection );
  • ve.ce.Surface.prototype#showSelection:3430
    • $( rangeSelection.start.node ).closest( '[contenteditable=true]' ).focus();

Change 195631 had a related patch set uploaded (by Mooeypoo):
Focus the surface after VE is already active

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

Change 195631 merged by jenkins-bot:
Focus the surface after VE is already active

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

Verified the fix in Betalabs