Adding a discussion to a user talk page should show the new topic without refresh
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	• Niedzielski
	Jan 5 2018, 2:05 PM

Description

There is a problem in the workflow to add a new talk topic to any talk page that is not a talk page for the NS_MAIN namespace.

Note: This seems to work fine on a talk page e.g. Talk:Foo but not a user talk page e.g. User talk:Foo or a module talk page e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Module talk:Foo#

Reproduction:

Visit a user talk page and tap "add discussion".
Enter some text and press save.
Refresh the page.

Expected

New topics are shown immediately.

Actual

The new topic is added but not shown until refresh.

en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox.png (1×1 px, 136 KB)

en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (1).png (1×1 px, 81 KB)

en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (4).png (1×1 px, 143 KB)

en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (3).png (1×1 px, 133 KB)

en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (1).png (1×1 px, 148 KB)

acceptance criteria

There is some logic inside resources/skins.minerva.talk/init.js to reload the page when a talk discussion is added (see inTalkNamespace variable). It doesn't seem to be running in any other namespace than namespace 2. Why?
Opportunistic refactoring - instead of using a global event the talk-added-wo-overlay event should be local to the current TalkOverlay.

developer notes

This will require changes in Minerva + MobileFrontend.

Details

	Subject	Repo	Branch	Lines +/-
	Passing currentPageTitle option to TalkSectionAddOverlay	mediawiki/skins/MinervaNeue	master	+2 -0
	Fix talk page refresh after adding content	mediawiki/extensions/MobileFrontend	master	+8 -5

Customize query in gerrit

Related Objects

Mentioned Here: T189194: Regression: Talk overlay is not showing on pages which do not yet have a talk page
T188553: Vector browser test blocking merge in Minerva

Event Timeline

• Niedzielski created this task.Jan 5 2018, 2:05 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 5 2018, 2:05 PM

• Niedzielski updated the task description. (Show Details)Jan 5 2018, 2:07 PM

ovasileva triaged this task as Medium priority.Jan 8 2018, 11:43 PM

Jdlrobson added projects: good first task, MinervaNeue.Jan 9 2018, 12:14 AM

Jdlrobson updated the task description. (Show Details)

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJan 9 2018, 12:14 AM

Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.Jan 9 2018, 12:14 AM

Jdlrobson renamed this task from Adding a discussion to a talk page should show the new topic without refresh to Adding a discussion to a user talk page should show the new topic without refresh.Jan 9 2018, 12:35 AM

Jdlrobson updated the task description. (Show Details)

Jdlrobson added projects: patch-welcome, Google-Code-in-2017.Jan 9 2018, 5:29 PM

ovasileva moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Jan 17 2018, 4:45 PM

Jdlrobson moved this task from Backlog to Bugs on the MinervaNeue board.Jan 20 2018, 12:34 AM

Jdlrobson set the point value for this task to 3.Feb 6 2018, 5:46 PM

Jdlrobson removed a project: good first task.

ovasileva added a project: Readers-Web-Kanbanana-Board-Old.Feb 21 2018, 4:04 PM

ovasileva moved this task from To Do to Needs Design Review on the Readers-Web-Kanbanana-Board-Old board.Feb 21 2018, 6:33 PM

ovasileva moved this task from Upcoming to 2017-18 Q3 on the Web-Team-Backlog board.Feb 22 2018, 11:49 AM

Jdrewniak claimed this task.Feb 26 2018, 11:23 AM

Jdrewniak moved this task from Needs Design Review to Doing on the Readers-Web-Kanbanana-Board-Old board.

Jdrewniak removed projects: Google-Code-in-2017, patch-welcome.

This looks like it's not just an issue on the user namespace. but on article name spaces as well.

The video below shows me creating a new discussion topic on the article namespace, from the "talk" page instead of the article page, and the same error occurs.
The new topic doesn't show up until the page is refreshed.

Editing or deleting a topic however, does seem to reload the content automatically.

(video link)
https://drive.google.com/file/d/14mKKNmkg1ifbNE56M8HQ00Q-GIPmWIeT/view?usp=sharing

Change 414986 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Fix talk page refresh after adding content

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

gerritbot added a project: Patch-For-Review.Feb 27 2018, 12:11 PM

the patch above represents the minimum viable fix. It fixes the bug but the resulting (and intended) behaviour is that the page does a hard refresh automatically, instead of the user having to hit refresh. The new content is not being shown "immediately" after save though. After looking through the code, I think this ideal behaviour can be achieved with some refactoring.

If TalkSectionAddOverlay inherited from EditorOverlayBase instead of TalkOverlayBase, it could take advantage of the onSaveComplete method that reloads the content without a hard refresh (as seen in this video after editing / deleting content) and we wouldn't need the custom logic to do the hard refresh. Also, I see a few FIXME's about using the gateways instead of editor api's directly. @Jdlrobson thoughts?

Jdlrobson moved this task from Doing to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Feb 27 2018, 5:09 PM

Change 415272 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Passing pageTitle option to TalkSectionAddOverlay

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

For future reference, I'm explaining the cause of this error.

There's a condition in the code that determines wether or not a user used the talk overlay or came from the talk page. That condition is based on comparing the value of the data-title attribute of the "add discussion" button to the title of the current page. The data-attribute of the button always points to the talk page, whereas the title of the current page could be just the article title (if the user is using the overlay).

The problem was that this.title in TalkSectionAddOverlay contains spaces but mw.config.get( 'wgPageName' ) which gets the title of the current page, contains underscores, so the comparison would always be false.

The fix was to use M.getCurrentPage().title instead of mw.config.get( 'wgPageName' ) for the page title comparison.

Jdrewniak moved this task from Needs More Work to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Feb 28 2018, 5:43 PM

I'll review

Jdlrobson claimed this task.Feb 28 2018, 6:10 PM

Jdlrobson added a subscriber: Jdrewniak.

Jdlrobson removed Jdlrobson as the assignee of this task.Feb 28 2018, 7:36 PM

Jdlrobson moved this task from Needs Code Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.

Change 414986 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix talk page refresh after adding content

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

Merging of https://gerrit.wikimedia.org/r/415272 blocked on T188553

Jdlrobson moved this task from Blocked on Others to Needs QA on the Readers-Web-Kanbanana-Board-Old board.Mar 1 2018, 6:18 PM

Change 415272 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Passing currentPageTitle option to TalkSectionAddOverlay

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

Can be tested on the beta cluster.

@Jdlrobson, I am still seeing this issue on Beta Cluster. Maybe beta hasn't updated yet. I tried a few talk pages including this one https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Sandbox

Looks like this needs a bit more investigation..

Looks like the beta cluster is running old code.
Can you test on reading web staging (word of warning: this is currently configured to use Hindi Wikipedia but that should not impact your testing)

Working for me on Staging

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-03-06 (1.31.0-wmf.24)).Mar 6 2018, 11:01 AM

Looks good to me. There's some things in the acceptance criteria I'm not sure I can sign off for. @Jdlrobson - could you take a look?

Jdlrobson claimed this task.Mar 7 2018, 6:04 PM

Jdlrobson added a subscriber: ABorbaWMF.

This seems to have a caused a regression, that's been picked up by our browser tests - it's now no longer possible to interact with a page which doesn't exist.

Browser tests have been failing for 5 days now. Not sure how we missed that :/

Jdlrobson moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.Mar 8 2018, 10:36 PM

The regression (T189194) was actually caused by another change, so this looks good. Sorry for jumping to conclusions.

	F12263093: en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (1).png
	Jan 5 2018, 2:07 PM

	F12262821: en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (4).png
	Jan 5 2018, 2:05 PM

	F12262813: en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox.png
	Jan 5 2018, 2:05 PM

	F12263007: en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (1).png
	Jan 5 2018, 2:05 PM

	F12262826: en.m.wiktionary.org_wiki_User_talk_Niedzielski_sandbox (3).png
	Jan 5 2018, 2:05 PM

	F14433579: image.png
	Mar 5 2018, 6:14 PM

Adding a discussion to a user talk page should show the new topic without refreshClosed, ResolvedPublic3 Estimated Story PointsActions