Page MenuHomePhabricator

Visual Editor removes user from ProofreadPage header
Closed, ResolvedPublic

Event Timeline

Mpaa created this task.Aug 18 2018, 3:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 18 2018, 3:25 PM
Mpaa triaged this task as Unbreak Now! priority.Aug 18 2018, 3:28 PM
Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptAug 18 2018, 3:28 PM

This took a while to find, because the issue only occurs when switching from wikitext to VE. I couldn't reproduce the problem when going from the view mode directly to VE. I reviewed the VE code for handling the page quality level thing and I'm confident it's handled correctly there.

But when switching from wikitext to VE, we do this:

// The user attribute is populated later
( !isNaN( level ) ? '<pagequality level="' + level + '" user="" />' : '' ) +
...

Unfortunately the user attribute is not populated later, so it stays empty (unless you change the page quality level in VE, then it's correctly set to your username).

This gives problems when page needs to change status in following edits (e.g. it is not possible to Validate a pae, see e.g Page:Carroll - Sylvie and Bruno Concluded.djvu/212 on en:wikisource.

Are you sure this is the case? I am not the most familiar with ProofreadPage, but as far as I can tell, in the worst case you will just need another user to set it instead of you.

matmarex claimed this task.Aug 20 2018, 9:13 PM
matmarex lowered the priority of this task from Unbreak Now! to Needs Triage.

I don't think this needed to be marked as "Unbreak Now!". The issue has existed for a while, and ProofreadPage is able to deal with missing or invalid usernames given in the "user" field (it must be – existing usernames already saved in existing content can become invalid, e.g. when the user is renamed).

But, since I already investigated it, there's not a lot of work to actually make the patch…

Mpaa added a comment.Aug 20 2018, 9:16 PM

Yes, that it is how I reproduced. To clarify: 1) I went to edit mode, wikitext by default, then w/o saving 2) I switched to VE, 3) I saved.

The problem is that when user="", nobody can change status, an edit must be made and saved, so that the User field is populated again, and then it is possible to change the status.

If there is a solution to such problem (that is, populate user when switching), it would be very much appreciated.

Mpaa triaged this task as Normal priority.Aug 20 2018, 9:17 PM
Mpaa raised the priority of this task from Normal to Needs Triage.

existing usernames already saved in existing content can become invalid, e.g. when the user is renamed).

Is it a bug that need to be solved ? Perhaps a task can be created for this.

Mpaa added a comment.Aug 20 2018, 9:21 PM

I don't think this needed to be marked as "Unbreak Now!". The issue has existed for a while, and ProofreadPage is able to deal with missing or invalid usernames given in the "user" field (it must be – existing usernames already saved in existing content can become invalid, e.g. when the user is renamed).
But, since I already investigated it, there's not a lot of work to actually make the patch…

Sorry about that, I marked it as 'Unbreak now!" as I initially thought it could happen at every edit. Only later on I realized it was happening under limited circumstances but I forgot to lower prio.
I agree is not such a high prio.

Change 454155 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/ProofreadPage@master] Don't clear pagequality 'user' attribute when switching from WTE to VE

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

existing usernames already saved in existing content can become invalid, e.g. when the user is renamed).

Is it a bug that need to be solved ? Perhaps a task can be created for this.

I don't think it is feasible to fix it. It would be relatively slow, compared to updating machine-readable data, like e.g. usernames in edit history. And we probably shouldn't modify the text of existing revisions, ever, anyway.

This is similar to usernames in signatures etc., which also don't get updated when renaming an user (unless someone runs a bot to do it, but then the username still remains unchanged in old revisions).

The proper solution to that problem would be to not store the page status information in the page text, but rather in some dedicated database table :)

Ankry added a comment.Aug 21 2018, 4:23 PM

The proper solution to that problem would be to not store the page status information in the page text, but rather in some dedicated database table :)

+1

Deskana triaged this task as Low priority.Aug 28 2018, 6:56 PM
Deskana moved this task from To Triage to Freezer on the VisualEditor board.Aug 29 2018, 1:22 PM

Change 454155 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@master] Don't clear pagequality 'user' attribute when switching from WTE to VE

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

matmarex closed this task as Resolved.Aug 30 2018, 9:37 PM
matmarex removed a project: Patch-For-Review.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptAug 30 2018, 9:37 PM