File History: Comments are not displayed all versions
Closed, ResolvedPublic

Description

This appears to be happening on multiple projects, including Commons and English Wikipedia. Latest discussion is at https://commons.wikimedia.org/w/index.php?title=Commons:Village_pump&oldid=258083317#File_History:_Comments_missing_for_non-current_versions

Jeff_G created this task.Sep 9 2017, 6:03 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 9 2017, 6:03 AM

Thanks for reporting this.
Please provide a test case.

Jeff_G added a comment.Sep 9 2017, 3:30 PM

It is not about the normal history[1], about the filehistory on the file page[2]

[1] https://commons.wikimedia.org/wiki/File:Example.jpg?action=history

[2] https://commons.wikimedia.org/wiki/File:Example.jpg#filehistory

Thank you for pointing that out, you are exactly right. The same field (here called Description), is showing up correctly on my list of files https://commons.wikimedia.org/w/index.php?title=Special:ListFiles/Jeff_G.&ilshowall=1 , so the data is not gone, it just appears to be hidden for some reason.

Aklapper renamed this task from File History: Comments missing for non-current versions to File History: Comments are not displayed anymore for non-current versions.Sep 9 2017, 3:32 PM

My case: uploaded a new version of an image, the comment for the previous version became absent.

Reedy added a subscriber: Reedy.Sep 9 2017, 4:00 PM

Did this start on Wednesday? When .17 was rolled out to commonswiki?

If so, it's very likely https://gerrit.wikimedia.org/r/#/c/357892/

Change 376880 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] LocalFile: Fix setting $this->description after CommentStore changes

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

Change 376915 had a related patch set uploaded (by Reedy; owner: Legoktm):
[mediawiki/core@wmf/1.30.0-wmf.17] LocalFile: Fix setting $this->description after CommentStore changes

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

Change 376880 merged by jenkins-bot:
[mediawiki/core@master] LocalFile: Fix setting $this->description after CommentStore changes

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

Change 376915 merged by jenkins-bot:
[mediawiki/core@wmf/1.30.0-wmf.17] LocalFile: Fix setting $this->description after CommentStore changes

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

Mentioned in SAL (#wikimedia-operations) [2017-09-09T20:41:45Z] <legoktm@tin> Synchronized php-1.30.0-wmf.17/includes/filerepo/file/LocalFile.php: Fix issues related to comments and files in UI and API - T175443 T175444 (duration: 00m 46s)

Legoktm closed this task as Resolved.Sep 9 2017, 8:44 PM
Legoktm claimed this task.
Legoktm added a subscriber: Legoktm.

This should be fixed now.

Thank you all!

Jeff_G added a comment.EditedSep 14 2017, 1:02 PM

Now, the opposite is happening, we are seeing comments for all the old versions of a file, but no comments for the current version. Please see https://commons.wikimedia.org/w/index.php?title=User_talk:Jeff_G.&diff=258558508&oldid=258188936 and https://commons.wikimedia.org/wiki/Commons:Village_pump#Disappearing_comment_in_.22File_history.22_section_for_the_current_version_of_a_file for details.

Jeff_G reopened this task as Open.Sep 14 2017, 2:41 PM
Reedy added a comment.Sep 14 2017, 4:31 PM

What happens if you purge the page? I noticed that fixed the issue when the first was still missing after the patch was deployed

Offnfopt added a comment.EditedSep 14 2017, 11:30 PM

When you purge, the comment for the current revision does come back, but after some time passes (a few minutes) it disappears again.

Speravir added a comment.EditedSep 15 2017, 4:20 PM

I confirm @Jdx' observation, cf, diff link:

A small "how to" replicate the problem:

Choose a random file – there is no comment for the current version (of course I assume that some comment exists).
Purge the file – the comment appears.
Press "reload" button in your browser – the comment disappears (I've noticed that sometimes the button has to be pressed twice).

“Random file” is linked to Commons:Special:Random/File.
Additional note: On reloading with cache bypassing the comment does not disappear.

Speravir added a subscriber: Jdx.Sep 15 2017, 4:36 PM
Reedy triaged this task as High priority.
Reedy renamed this task from File History: Comments are not displayed anymore for non-current versions to File History: Comments are not displayed all versions.Sep 18 2017, 12:03 AM
Restricted Application added projects: Commons, Multimedia. · View Herald TranscriptSep 18 2017, 12:03 AM

Change 376880 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] LocalFile: Fix setting $this->description after CommentStore changes

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

https://gerrit.wikimedia.org/r/#/c/377276/ was a followup

I confirm @Jdx' observation, cf, diff link:

A small "how to" replicate the problem:

Choose a random file – there is no comment for the current version (of course I assume that some comment exists).
Purge the file – the comment appears.
Press "reload" button in your browser – the comment disappears (I've noticed that sometimes the button has to be pressed twice).

It doesn't matter how many times I reload (cmd + r) the page, it doesn't disappear again for me. But appending a query string like ?foo will make it go again on that new pageload

So digging into it further.

If I stick a var_dump( $fields, $cacheVal ); die(); at line 300 of LocalFile.php

/var/www/wiki/mediawiki/core/includes/filerepo/file/LocalFile.php:300:
array (size=12)
  0 => string 'size' (length=4)
  1 => string 'width' (length=5)
  2 => string 'height' (length=6)
  3 => string 'bits' (length=4)
  4 => string 'media_type' (length=10)
  5 => string 'major_mime' (length=10)
  6 => string 'minor_mime' (length=10)
  7 => string 'metadata' (length=8)
  8 => string 'timestamp' (length=9)
  9 => string 'sha1' (length=4)
  10 => string 'user' (length=4)
  11 => string 'user_text' (length=9)
/var/www/wiki/mediawiki/core/includes/filerepo/file/LocalFile.php:300:
array (size=13)
  'fileExists' => boolean true
  'size' => int 22960
  'width' => int 800
  'height' => int 600
  'bits' => int 8
  'media_type' => string 'BITMAP' (length=6)
  'major_mime' => string 'image' (length=5)
  'minor_mime' => string 'jpeg' (length=4)
  'metadata' => string 'a:5:{s:11:"XResolution";s:10:"95961/1000";s:11:"YResolution";s:10:"95961/1000";s:14:"ResolutionUnit";i:2;s:8:"Software";s:16:"Paint.NET v3.5.8";s:22:"MEDIAWIKI_EXIF_VERSION";i:2;}' (length=179)
  'timestamp' => string '20110411222941' (length=14)
  'sha1' => string '0jo9fquqa3mohi5ruzefcb4qmkefoyk' (length=31)
  'user' => string '3' (length=1)
  'user_text' => string 'LolTestUser' (length=11)

So we're not getting description out of it

The reason why, is we're passing a prefix of '' to getCacheFields - https://github.com/wikimedia/mediawiki/blob/master/includes/filerepo/file/LocalFile.php#L277

So we're not caching the description, and we're not getting it again either

So https://gerrit.wikimedia.org/r/#/c/376915 isn't completely right

Change 378651 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Return description fields for unprefixed image cache rows

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

Reedy added a comment.Sep 18 2017, 1:04 AM

Patch above tested, and it fixes the issue. A final purge is needed to get the description cached again...

Probably should bump LocalFile::VERSION to save having to purge everything

Change 378706 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Bump LocalFile::VERSION to invalidate file page cache

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

Change 378712 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@wmf/1.30.0-wmf.18] Return description fields for unprefixed image cache rows

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

Change 378712 merged by jenkins-bot:
[mediawiki/core@wmf/1.30.0-wmf.18] Return description fields for unprefixed image cache rows

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

Mentioned in SAL (#wikimedia-operations) [2017-09-18T14:27:38Z] <reedy@tin> Synchronized php-1.30.0-wmf.18/includes/filerepo/file/LocalFile.php: T175444 (duration: 00m 47s)

Reedy added a comment.Sep 18 2017, 2:31 PM

Mentioned in SAL (#wikimedia-operations) [2017-09-18T14:27:38Z] <reedy@tin> Synchronized php-1.30.0-wmf.18/includes/filerepo/file/LocalFile.php: T175444 (duration: 00m 47s)

Ok, this seems to have fixed it. You will need to purge the page (for the moment), if the description is missing from the file history table.

We should get the two patches merged into master, and then when .19 comes through, it should fix things up completely, without causing too much cache problems from invalidating all the localfile caches in one go...

Change 378651 merged by jenkins-bot:
[mediawiki/core@master] Return description fields for unprefixed image cache rows

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

Ramsey-WMF moved this task from Untriaged to Tracking on the Multimedia board.Sep 18 2017, 4:38 PM

Change 378706 merged by jenkins-bot:
[mediawiki/core@master] Bump LocalFile::VERSION to invalidate file page cache

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

Reedy closed this task as Resolved.Sep 20 2017, 1:42 PM

Resolving off again. Not heard any comments either way. Feel free to leave further comments and/or reopen if it turns out it's not fixed