Page MenuHomePhabricator

Restore section hash fragment after section editing in mobile VE
Closed, ResolvedPublic

Description

Restoring the section hash fragment will scroll the user back to the section they just edited, showing them their changes.

We do this on desktop, but it doesn't work on mobile, probably due to MobileFrontend's handling for hash fragments.

Event Timeline

Esanders created this task.Jan 7 2019, 10:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 7 2019, 10:50 PM
Esanders claimed this task.Jan 16 2019, 6:48 PM

Change 484518 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] Redirect after mobile section edit

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

I reviewed the patch and I don't like it. It currently also changes the behavior for aborting an edit, and I think the current one was just fine.

(Now it only changes the behavior after saving, rather than aborting.)

DannyH removed a subscriber: DannyH.Feb 27 2019, 8:22 PM

Change 484518 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Redirect to the section after mobile section edit (visual)

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

Yeah, this works on mobile now but not on desktop anymore, for which there is a bug already reported.

Yeah, this works on mobile now but not on desktop anymore, for which there is a bug already reported.

Do you mean T217269: [Regression wmf.19] Adding a new section does not scroll to the bottom of the page anymore or is there another, more severe bug here? (Assuming that one only applies to adding new sections at the end, rather than editing existing sections.)

Yeah, this works on mobile now but not on desktop anymore, for which there is a bug already reported.

Do you mean T217269: [Regression wmf.19] Adding a new section does not scroll to the bottom of the page anymore or is there another, more severe bug here? (Assuming that one only applies to adding new sections at the end, rather than editing existing sections.)

Ah that's right! The reported one is for adding new section, it works fine for editing existing ones on desktop. Ignore my last comment :) Sorry for the confusion.

ppelberg closed this task as Resolved.Mar 14 2019, 10:54 PM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMar 14 2019, 10:54 PM
Ryasmeen reopened this task as Open.Mar 15 2019, 7:52 PM

This is broken now again! Sigh! :(

Ryasmeen added a comment.EditedMar 15 2019, 9:56 PM

Here is the screen capture:

I note that @Ryasmeen changed the title of the section. Perhaps we're generating an outdated section-hash as a result rather than one based on the new hash for the new title? The video didn't show the URL, so I can't be sure.

@DLynch: Yeah you're right, after changing the title of that section and saving it, the URL shows the old section name: https://en.m.wikipedia.org/wiki/User:RYasmeen_(WMF)/sandbox#new_section instead of the new one.

But interestingly this also happens even if I don't change the title of that section but the content under it, on wikis with section editing enabled. Checked on bn.wiki on my sandbox: https://bn.m.wikipedia.org/wiki/ব্যবহারকারী:RYasmeen (WMF)/খেলাঘর

Here is the screen capture for this one showing the URL change:

Okay, yeah. Patch made it so that we do var fragment = this.getSectionFragmentFromPage(); but that just interrogates the current markup of the page pre-edit. This explains changing the section-name breaking things, since after the page-reload it'll be a different id to look for.

Not sure what difference section-editing makes. Eyeballing that fragment, it looks like it should be correct, though I suppose there's room for section editing to have changed the behavior of fragments on the page. I'll need to go in and actually compare the post-save id attribute on a section-header to see what's differing.

Should we make a new ticket for these much-more-specific sub-issues?

Change 499259 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: When saving a section don't trust current page markup

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

Change 499259 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: When saving a section don't trust current page markup

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

This is really weird, I tested this on Beta cluster, test.wiki, en.wiki and it worked in all three environments, except it's still not working for bn.wiki. I have no idea what it is that I am doing different for this wiki :(

DLynch added a comment.Apr 4 2019, 9:57 PM

@Ryasmeen -- honestly, the weird part there is that it worked on en.wiki. We have a train stall at the moment, so en.wiki should still be on 1.33.0-wmf.23 which doesn't have the latest patch.

@Ryasmeen -- honestly, the weird part there is that it worked on en.wiki. We have a train stall at the moment, so en.wiki should still be on 1.33.0-wmf.23 which doesn't have the latest patch.

lol. I think I know what happened, I checked the special:version page on en.wiki and it was on wmf.24 but I think by the time I checked on bn.wiki it was reverted back to wmf.23. I will check it again later :)

ok, this time it's real. Still seeing this on bn.wiki. Here is the video capture:

DLynch added a comment.Apr 9 2019, 1:34 PM

Okay, that's a completely separate issue. Seems to be with MobileFrontend not handling non-latin fragments at all well for the initial pageload. I'll make a different ticket.

I see, alright I am closing this then.

ppelberg closed this task as Resolved.May 24 2019, 11:11 PM
ppelberg reopened this task as Open.EditedOct 5 2019, 7:22 PM

ok, this time it's real. Still seeing this on bn.wiki. Here is the video capture:

I seem to be encountering the same issue Rummana encountered above:

Actual behavior

  1. Edit the body of a section
  2. Publish changes

⚠️ 3. View is scrolled to the top of the article in read mode

Expected behavior

  1. Edit the body of a section
  2. Publish changes

✅ 3. View is scrolled to the top of the section I was just editing

Configuration
Browser: Safari
Device: iPhone Xs
OS: iOS 13.1.2

Video
https://youtu.be/fikbhfZjmJE

ok, this time it's real. Still seeing this on bn.wiki. Here is the video capture:

I seem to be encountering the same issue Rummana encountered above:

Actual behavior

  1. Edit the body of a section
  2. Publish changes

⚠️ 3. View is scrolled to the top of the article in read mode

Expected behavior

  1. Edit the body of a section
  2. Publish changes

✅ 3. View is scrolled to the top of the section I was just editing

Configuration
Browser: Safari
Device: iPhone Xs
OS: iOS 13.1.2

Video
https://youtu.be/fikbhfZjmJE

Yes, I am getting this too.

matmarex closed this task as Resolved.Oct 7 2019, 11:58 PM

I think this is a new problem. We fixed this back in April, and it has only regressed recently. It'll be more convenient to investigate in a new task, so I filed one: T234868

I think this is a new problem. We fixed this back in April, and it has only regressed recently. It'll be more convenient to investigate in a new task, so I filed one: T234868

Got it. Thanks for doing that, Bartosz.