Page MenuHomePhabricator

Page scrolls to the wrong place after posting a comment
Closed, ResolvedPublic


@Ciell reports that this reply scrolled to the wrong place on the page after posting her reply.

Perhaps it happens more often if there are ===Level 3 section headings===, collapsed content, long pages???

Event Timeline

This might have an easy explanation: when using the reply tool on a diff page, after publishing the reply the updated page replaces the entire page content, including the part where the diff is shown. So in fact the vertical scroll position doesn't change, but a 'jump' of the height of the disappeared parts (diff table, diff-currentversion-title h2, probably some more elements) can be perceived.

(Of course I can't be certain if this was what happened in the reported case, but it is easy to replicate.)

Solution (based on observation of what happens when publishing a reply):
Instead of replacing the content of div#mw-content-text with the new, replace (the content of) the existing

Steps to replicate this:

  1. open a diff page (the page content must be long enough compared to the height of the diff to return at the exact same position)
  2. scroll just a few pixels from the top to an easy to remember position
  3. page-down enough pages to see the first reply links (or maybe one more: assure that there is enough room for the reply widget)
  4. use the reply tool to post a bold statement (observe the jump after publishing)
  5. page-up the same number of pages as in step 3 (notice the disappeared diff)
  6. observe the exact same position as in step 2

Easy link to a test diff page:

We actually don't scroll at all when replying – but if the content above your comment changes (e.g. due to collapsible navboxes expanding/collapsing themselves, or somebody else commenting or archiving comments while you're replying, or indeed due to the diff disappearing – I didn't think of that one), your comment might no longer be in view. We should probably scroll to it when that happens.

So it turns out there's a little bit of scrolling happening:

When you save your comment the widget is torn down, and during that the CommentController re-focuses the 'Reply' link you opened your comment with. That causes the reply link that launched your comment to centred in the view (browser dependent probably). As BD points out, we then update the page and if there are other changes you may not be in the right place.

In addition to scrolling a posted comment into view, I think the re-focussing should only happen if you cancel out of the tool.

Change 604791 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/DiscussionTools@master] Only re-focus the reply link when reply is abandoned

Change 604792 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/DiscussionTools@master] Scroll the after-reply highlight into view

Change 604791 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Only re-focus the reply link when reply is abandoned

Waiting for OOUI release.

@Volker_E do you know what date the next OOUI is scheduled for? ...we're trying to figure out what the proper place for this ticket is.

OOUI was released today, now we need to merge our patch.

Change 604792 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Scroll the after-reply highlight into view

This patch improves the behavior and should fix e.g. the case with diffs described by @Marc above, but it doesn't fix other cases, e.g. with a collapsed table of contents. Here's an example, this is with @Esanders's patch applied:

The problem is that after the page is updated to display your new comment, DiscussionTools initializes before everything else, including before the table of contents collapses, so we highlight the comment in the wrong spot and also scroll to the wrong spot.

Change 607619 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Another attempt to fix page re-initialization after updating

Change 607619 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Another attempt to fix page re-initialization after updating

ppelberg added a project: Skipped QA.