RevisionSlider fails to re-trigger the Thanks JS code when the diff changes
Closed, ResolvedPublic5 Story Points

Description

  1. Go to a diff.
  2. Click Thanks; see that the JS loads inline, asking if you're sure.
  3. Use RevisionSlider to switch to a different diff.
  4. Click Thanks; See that instead of loading inline, it takes you to the non-JS dedicated Special:Thanks/… page.

There's possibly other JS that loads on diff pages which also should be triggered.

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

We probably need an mw.hook thing that thanks can listen to?

We probably need an mw.hook thing that thanks can listen to?

We don't have one already?

There is mw.hook( 'wikipage.diff' ).fire( $wikiDiff.find( 'table.diff' ) );, but that runs on preview, which we don't put thanks links up for...

WMDE-leszek set the point value for this task to 5.Aug 11 2016, 2:57 PM
WMDE-leszek moved this task from Proposed to Backlog on the TCB-Team-Sprint-2016-08-11 board.
WMDE-leszek triaged this task as High priority.Aug 11 2016, 3:13 PM

I had a quick look around and couldn't really find a specific diff hook to be firing!

We do currntly fire the following on reload:

mw.hook( 'wikipage.content' ).fire( $contentText );

And we could always introduce another hook?

Addshore moved this task from Incoming to Revision Slider on the TCB-Team board.Aug 13 2016, 8:34 AM
Lea_WMDE moved this task from Incoming to Doing on the Revision-Slider board.Aug 15 2016, 12:44 PM

Change 306677 had a related patch set uploaded (by WMDE-leszek):
Add an extension-specific hook for the diff reload event

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

Change 306679 had a related patch set uploaded (by WMDE-leszek):
Re-add actions to Thanks links when Revision Slider reloads a diff

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

Instead of an extension specific one, can we have a generic hook that is triggered on diff page views by core, and then revisionslider can update it when it changes the diff?

Change 306679 merged by jenkins-bot:
Re-add actions to Thanks links when Revision Slider reloads a diff

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

Change 307316 had a related patch set uploaded (by WMDE-leszek):
Add a hook trigged when the diff on the diff page changes

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

Change 307317 had a related patch set uploaded (by WMDE-leszek):
Re-add actions to Thanks links when a diff on the diff page changes

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

@Legoktm yes, this makes sense.
I've submitted https://gerrit.wikimedia.org/r/307316 and https://gerrit.wikimedia.org/r/307317 then, and adjusted https://gerrit.wikimedia.org/r/#/c/306677/. I am still not sure about name of the new hook and if it was added to a right place (in all those resource subdirs structure etc).

Change 310229 had a related patch set uploaded (by Addshore):
Listen to the wikipage.diff hook for adding JS links

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

Change 310231 had a related patch set uploaded (by Addshore):
Fire wikipage.diff instead of revslider.diffreload

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

Change 310232 had a related patch set uploaded (by Addshore):
Stop listening to revslider.diffreload hook

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

Change 310229 merged by jenkins-bot:
Listen to the wikipage.diff hook for adding JS links

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

Change 310231 merged by jenkins-bot:
Fire wikipage.diff instead of revslider.diffreload

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

Change 310232 merged by jenkins-bot:
Stop listening to revslider.diffreload hook

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

Change 307316 abandoned by WMDE-leszek:
Add a hook trigged when the diff on the diff page changes

Reason:
What Krinkle said makes perfect sense, and there is no need to add a new hook.

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

Change 307317 abandoned by WMDE-leszek:
Re-add actions to Thanks links when a diff on the diff page changes

Reason:
New core hook is not needed what makes this change foobar

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

Change 306677 abandoned by WMDE-leszek:
Trigger the hook when the diff content changes

Reason:
Superseded by Ie88021abb2325cc6259cf2fb041fbdca4ae9ca89

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

Tobi_WMDE_SW moved this task from Doing to Done on the Revision-Slider board.Sep 27 2016, 10:16 AM
Tobi_WMDE_SW closed this task as Resolved.Sep 27 2016, 3:15 PM
Addshore reopened this task as Open.Jan 31 2017, 4:15 PM

It looks like this is happening again and needs some more investigation.

I have just confirmed this on testwiki and mw.org

Restricted Application added a project: User-Addshore. · View Herald TranscriptJan 31 2017, 4:15 PM
Addshore removed Addshore as the assignee of this task.

@Addshore, remember I told you months ago that the hook does not always work? For WikEdDiff and for thanks for example. I can see you recognized it too.

Addshore moved this task from Done to Backlog on the Revision-Slider board.Jan 31 2017, 4:33 PM
Lea_WMDE moved this task from Backlog to TODO on the Revision-Slider board.Apr 7 2017, 5:05 PM
WMDE-Fisch claimed this task.

Change 347822 had a related patch set uploaded (by WMDE-Fisch):
[mediawiki/extensions/RevisionSlider@master] Fix JS trigger for the thanks links

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

Change 347822 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@master] Fix JS trigger for the thanks links

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

Jdforrester-WMF closed this task as Resolved.Apr 12 2017, 3:58 PM

Confirmed working again; thank you!

Addshore moved this task from Backlog to Done on the User-Addshore board.Apr 13 2017, 3:24 PM