Page MenuHomePhabricator
Create Task
Maniphest T272438

Previous revision shown below when viewing a TimedText diff
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to reproduce:

Actual results:

  • Although the main diff table shows you the diff of the revision you're seeing, the rest of the page is treated as though you're viewing the previous revision, including the preview beneath the diff and the notice at the top, and clicking on the Edit tab will send you to editing the previous revision.

Expected results:

  • The diff page shows the correct (more recent) revision.

@Izno commented:

I would guess this is due to the Revision.php work done in 1.35.

Details

SubjectRepoBranchLines +/-
TimedText: Diff previews should show the new revisionmediawiki/extensions/TimedMediaHandlerREL1_35+1 -1
TimedText: Diff previews should show the new revisionmediawiki/extensions/TimedMediaHandlerREL1_39+1 -1
TimedText: Diff previews should show the new revisionmediawiki/extensions/TimedMediaHandlerREL1_38+1 -1
TimedText: Diff previews should show the new revisionmediawiki/extensions/TimedMediaHandlermaster+1 -1
TimedText: Diff previews should show the correct revisionmediawiki/extensions/TimedMediaHandlerREL1_35+1 -1
TimedText: Diff previews should show the correct revisionmediawiki/extensions/TimedMediaHandlerREL1_38+1 -1
TimedText: Diff previews should show the correct revisionmediawiki/extensions/TimedMediaHandlerREL1_39+1 -1
TimedText: Diff previews should show the correct revisionmediawiki/extensions/TimedMediaHandlermaster+1 -1
Customize query in gerrit

Event Timeline

Nardog created this task.Jan 20 2021, 12:37 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 20 2021, 12:37 AM
Nardog updated the task description. (Show Details)Jan 20 2021, 12:38 AM
Reedy renamed this task from Diff for TimedText is broken to Page version displayed on TimedText diff is incorrect.Jan 20 2021, 3:05 AM
Nardog renamed this task from Page version displayed on TimedText diff is incorrect to Wrong revision shown on TimedText diff.Jan 20 2021, 6:02 PM
Aklapper changed the task status from Open to Stalled.Jan 20 2021, 6:25 PM

Hi @Nardog, thanks for taking the time to report this! I fail to see a link called "Edit" on https://commons.wikimedia.org/wiki/Special:RecentChanges?hidenewpages=1&hidecategorization=1&hideWikibase=1&hidelog=1&namespace=102&limit=50&days=7&urlversion=2 and it's unclear to me where exactly after which steps to see a problem. Could you please follow the structure on https://www.mediawiki.org/wiki/How_to_report_a_bug and 1) provide a list of clear steps to reproduce, 2) what you expected where exactly, and 3) what you see instead, in separate sections? Thanks a lot.

foo-X.png (990×1 px, 278 KB)

Nardog updated the task description. (Show Details)Jan 20 2021, 6:40 PM

@Aklapper Is it clearer now?

Aklapper renamed this task from Wrong revision shown on TimedText diff to Wrong (previous) revision shown when opening a TimedText diff.Jan 20 2021, 6:52 PM
Aklapper changed the task status from Stalled to Open.
Aklapper added projects: MediaWiki-Core-Revision-backend, Regression.

Yes! Thanks a lot! :)

Nardog added subscribers: DannyS712, • brooke, Krinkle and 3 others.Jan 28 2023, 2:30 PM

@TheDJ @Reedy @Jdforrester-WMF @Krinkle @DannyS712 @brion Can someone take a look at this. "Show the content of the new revision" is like the job the diff page is supposed to do, and it has failed for years.

gerritbot added a comment.Jan 29 2023, 7:11 PM

Change 884461 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@master] TimedText: Diff previews should show the correct revision

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

gerritbot added a project: Patch-For-Review.Jan 29 2023, 7:11 PM
TheDJ claimed this task.Jan 29 2023, 7:11 PM
gerritbot added a comment.Jan 29 2023, 8:16 PM

Change 884461 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] TimedText: Diff previews should show the correct revision

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

Maintenance_bot removed a project: Patch-For-Review.Jan 29 2023, 8:30 PM
ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.21; 2023-01-30).Jan 29 2023, 9:00 PM
TheDJ closed this task as Resolved.Jan 29 2023, 9:01 PM
gerritbot added a comment.Jan 29 2023, 9:40 PM

Change 884345 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_39] TimedText: Diff previews should show the correct revision

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

gerritbot added a project: Patch-For-Review.Jan 29 2023, 9:40 PM

Change 884486 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_38] TimedText: Diff previews should show the correct revision

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

gerritbot added a comment.Jan 29 2023, 9:42 PM

Change 884487 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_35] TimedText: Diff previews should show the correct revision

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

gerritbot added a comment.Jan 29 2023, 9:46 PM

Change 884345 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_39] TimedText: Diff previews should show the correct revision

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

gerritbot added a comment.Jan 29 2023, 9:46 PM

Change 884486 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_38] TimedText: Diff previews should show the correct revision

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

gerritbot added a comment.Jan 29 2023, 9:48 PM

Change 884487 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_35] TimedText: Diff previews should show the correct revision

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

Maintenance_bot removed a project: Patch-For-Review.Jan 29 2023, 10:30 PM
Nardog awarded a token.Jan 30 2023, 12:55 AM
Nardog added a comment.Jan 30 2023, 5:51 PM

Doesn't appear to be fixed on Beta Cluster. Is this simply due to caching?

https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=TimedText%3AFolgers.beta.ogv.en.srt&diff=next&oldid=108645

Nardog reopened this task as Open.Feb 2 2023, 1:01 AM

Reopening because it is still reproducible on Commons (example) even though TimedMediaHandler there has been updated. @TheDJ @Reedy Can you please look into it again?

Reedy renamed this task from Wrong (previous) revision shown when opening a TimedText diff to Previous revision shown below when viewing a TimedText diff.Feb 2 2023, 1:03 AM
TheDJ added a comment.EditedFeb 2 2023, 10:23 AM

hmm, this is pretty strange.. Is the hook simply receiving the wrong content then ? That would mean this is a core problem..

Broken:
http://localhost/wiki/index.php?title=TimedText:Folgers-local.ogv.en.srt&curid=333&diff=725&oldid=720

Not broken:
http://localhost/wiki/index.php?title=TimedText:Folgers-local.ogv.en.srt&diff=prev&oldid=725

Commons from RC: https://commons.wikimedia.org/w/index.php?title=TimedText%3ATraiasca_Regele%21.ogg.en.srt&curid=128195256&diff=729416138&oldid=728560389&diffmode=source

Commons from history page: https://commons.wikimedia.org/w/index.php?title=TimedText%3ATraiasca_Regele%21.ogg.en.srt&diff=prev&oldid=729416138&diffmode=source

Looks to me like the hook is always receiving the 'oldid' content, even though oldid is not always the left side of the diff... (sigh how did we ever break the oldid param so badly?)

gerritbot added a comment.Feb 2 2023, 10:29 AM

Change 885996 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@master] TimedText: Diff previews should show the new revision

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

gerritbot added a project: Patch-For-Review.Feb 2 2023, 10:29 AM
TheDJ added a comment.EditedFeb 2 2023, 10:31 AM

I had misunderstood the hook. I thought it delivered the output with the appropriate context and you could overwrite it, but inspection of the implementation reveals that it is basically a hook BEFORE the context of the content is set. Makes sense in hindsight.

gerritbot added a comment.Feb 2 2023, 11:36 AM

Change 885996 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:37 AM

Change 885937 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_39] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:38 AM

Change 885938 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_38] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:39 AM

Change 885939 had a related patch set uploaded (by Reedy; author: TheDJ):

[mediawiki/extensions/TimedMediaHandler@REL1_35] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:45 AM

Change 885938 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_38] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:46 AM

Change 885937 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_39] TimedText: Diff previews should show the new revision

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

gerritbot added a comment.Feb 2 2023, 11:47 AM

Change 885939 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_35] TimedText: Diff previews should show the new revision

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

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.22; 2023-02-06); removed MW-1.40-notes (1.40.0-wmf.21; 2023-01-30).Feb 2 2023, 12:00 PM
Maintenance_bot removed a project: Patch-For-Review.Feb 2 2023, 12:31 PM
Nardog closed this task as Resolved.Feb 2 2023, 11:51 PM

Working as expected on Beta Cluster (example). Thank you!

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL