Page MenuHomePhabricator

Editing a page with unflagged changes should edit latest version in VE, even if you view the stable version
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. Log out (or use a private window).
  2. Find a page with unflagged changes, https://de.wikipedia.org/wiki/Spezial:Letzte_%C3%84nderungen will have some.
  3. Note that you are shown the stable version, not the latest revision.
  4. Click the tab for source editing "Quelltext bearbeiten", note that you are editing the latest revision.
  5. Go back, and click the tab for editing in VE "Bearbeiten".

Expected: As for source editing, you should edit the latest revision.
Actual: You are editing the stable version. If you don't notice the warning you are likely to accidentally revert all changes not yet flagged.

Event Timeline

Schnark created this task.May 15 2017, 8:08 AM
Restricted Application added a project: VisualEditor. · View Herald TranscriptMay 15 2017, 8:08 AM
Restricted Application added subscribers: TerraCodes, Aklapper. · View Herald Transcript
Deskana triaged this task as Normal priority.Aug 1 2017, 7:25 PM
Deskana moved this task from To Triage to TR1: Releases on the VisualEditor board.
clel added a subscriber: clel.Aug 20 2017, 8:59 AM

Commenting to give this more relevance. Also: Is there a warning that there are changes that will be overwritten?

@clel: There's a "Attention!" note when opening the editor, stating that you're editing an older version, which will become the current version after saving. It also says that clicking the "Edit" button again would fix this issue. However, clicking this button does not have any effect at all. After closing this note, there's no further warning message when saving your changes.


This is the only info you get. After switching to English I noticed this text is actually better, mentioning the possible data loss explicitly (other than the German text), and not including the strange advice of clicking "Edit" again. However, if you click anywhere in the text, the note is gone, and there's no further warning when saving the page.

clel added a comment.Aug 23 2017, 9:07 AM

@Tkarcher Thanks. Also I think that one has a chance to see the problem in the diff-view before saving, right? But nonetheless this should be fixed.

Arbnos added a subscriber: Arbnos.Nov 10 2017, 12:38 AM
Sunpriat2 added a subscriber: Sunpriat2.
stjn added a subscriber: stjn.

I would ask Editing team to look into this better: it is a very frequent bug appearing for anonymous editors in Russian Wikipedia. We have more than 7,000 stabilised articles (mainly the featured and good articles) and in each of those there were two different links on the stable version that is shown to anonymous viewers: VE link to old (stable) version and default editor link to last version. VE link should not differ from the link that is shown for default editor, and we even submitted a local fix for that (I suggest doing the same to other projects before it could be fixed) given how frequent this leads to deletion of edits due to not noticing that they edit the stable version and not the last version. Before VE was introduced, there were no such issues, so I am considering this a quite big regression.

@clel: There's a "Attention!" note when opening the editor, stating that you're editing an older version, which will become the current version after saving. It also says that clicking the "Edit" button again would fix this issue. However, clicking this button does not have any effect at all. After closing this note, there's no further warning message when saving your changes.

I would suggest that part of the issue is that the editnotices in VE are not displayed good enough and people just ignore them since it’s too simple to close that popup. I honestly see no reason why it couldn’t be made like this so it would be prominent enough to not to ignore:

Cirdan added a subscriber: Cirdan.Mar 5 2018, 10:47 PM
Cirdan added a comment.Mar 6 2018, 3:12 PM

This problem came up again on the German-language Wikipedia. Is there any chance that this will be fixed soon? Or is there an underlying technical difficulty?

clel added a comment.Mar 6 2018, 7:18 PM

I guess there is an underlaying lacking manpower, maybe combined with wrong priorities/management on the developer/bug team side :)

Deskana added a subscriber: Deskana.Mar 6 2018, 8:21 PM

I guess there is an underlaying lacking manpower, maybe combined with wrong priorities/management on the developer/bug team side :)

Yes, there are resource limitations, and that means that we have to say no to a lot of things, and that even very valid bugs like this one can not always be worked on. It's a bit mean to call that "wrong priorities/management". I try my best.

This problem came up again on the German-language Wikipedia. Is there any chance that this will be fixed soon? Or is there an underlying technical difficulty?

I will get someone to take a look at what might be going wrong here.

Deskana edited projects, added VisualEditor (Current work); removed VisualEditor.
clel added a comment.Mar 7 2018, 1:54 PM

Yes, there are resource limitations, and that means that we have to say no to a lot of things, and that even very valid bugs like this one can not always be worked on. It's a bit mean to call that "wrong priorities/management". I try my best.

Was maybe a bit too harsh, sorry. It is just a bit frustrating, when bugs that can probably be fixed within an hour are open that long or also projects like the one for finally having threaded discussions (https://www.mediawiki.org/wiki/Structured_Discussions) for easier use and comment tracking are failing due to them lacking basic features, which are technically totally possible and easy to implement, but just not there. In the meantime this project has now three? attempts which I consider a waste of work, since developing three different prototypes without productive functionality instead of one working product looks ridiculous in a way (also here sorry for being harsh). So if this really is only due to lacking manpower: How about hiring more developers, call for help (you call for money every year, so why not call for help as well) and maybe switch to a platform like GitHub to make contributing easier?

Was maybe a bit too harsh, sorry. It is just a bit frustrating, when bugs that can probably be fixed within an hour are open that long

This represents a fundamental misunderstanding of how prioritisation works. Firstly, it is a mistake to try guessing how difficult a bug is to fix. How simple bugs seem to you or I often does not correspond to the engineering difficulty. Secondly, how easy or hard something is to fix is only one of many factors which affect the prioritisation of a task. My team does not have the time to fix every bug that is reported, even if people make guesses that they're easy. This means that very valid bugs get put in the backlog.

or also projects like the one for finally having threaded discussions (https://www.mediawiki.org/wiki/Structured_Discussions) for easier use and comment tracking are failing due to them lacking basic features, which are technically totally possible and easy to implement, but just not there. In the meantime this project has now three? attempts which I consider a waste of work, since developing three different prototypes without productive functionality instead of one working product looks ridiculous in a way (also here sorry for being harsh). So if this really is only due to lacking manpower: How about hiring more developers, call for help (you call for money every year, so why not call for help as well) and maybe switch to a platform like GitHub to make contributing easier?

I have a specific role as the product manager for the Editing team. I can't tell other teams what they should and should not be working on. I can't tell the engineers of other teams to work for me instead of the products they're working on. I don't have any control over recruitment. I do not control the Wikimedia Foundation budget. I am not involved in fundraising. Even if I agree with everything you say, I have no control over any of the things you're talking about. Please give this feedback in the proper venues to people that can act on it, not in a bug report to a single product manager.

clel added a comment.Mar 7 2018, 2:51 PM

This represents a fundamental misunderstanding of how prioritisation works. Firstly, it is a mistake to try guessing how difficult a bug is to fix. How simple bugs seem to you or I often does not correspond to the engineering difficulty. Secondly, how easy or hard something is to fix is only one of many factors which affect the prioritisation of a task. My team does not have the time to fix every bug that is reported, even if people make guesses that they're easy. This means that very valid bugs get put in the backlog.

I only said that there is maybe wrong prioritisation. After you explained that there is none, at least at your "level" (I still think the teams might be wrong priorised), it still is frustrating to me. That is what I wanted to say.

I have a specific role as the product manager for the Editing team. I can't tell other teams what they should and should not be working on. I can't tell the engineers of other teams to work for me instead of the products they're working on. I don't have any control over recruitment. I do not control the Wikimedia Foundation budget. I am not involved in fundraising. Even if I agree with everything you say, I have no control over any of the things you're talking about. Please give this feedback in the proper venues to people that can act on it, not in a bug report to a single product manager.

First, I did not know that you there were different "areas" within the Wikipedia dev teams. Still, when you lack developers to fix all important bugs (I consider this important) in time, have you complained about it to the people above you and proposed some steps to improve it? I am only some guy from the internet, so I fear that my proposals won't get taken serious from the people responsible for it.

clel added a subscriber: jeremyb.Mar 22 2018, 2:56 PM

Ping to @jeremyb as I saw your activity on https://phabricator.wikimedia.org/T66281 and this seems to be somehow the same with its ability to produce data loss. However I have to say that the user will probably be warned by the diff displayed.

VE decides which revision to load based on the wgRevisionId config variable. When viewing the latest stable version (e.g. https://test2.wikipedia.org/wiki/VE-flaggedrevs – example from the merged task), this variable is set to the revision ID of the version actually being viewed, rather than the latest unflagged version (which is reasonable).

FlaggedRevs would have to give us a machine-readable way of detecting this case (perhaps with another config variable), so that we could decide to load the latest revision instead (wgCurRevisionId).

Wostr added a subscriber: Wostr.Apr 17 2018, 2:30 PM
Zache added a subscriber: Zache.Apr 17 2018, 3:03 PM

@matmarex There is wgStableRevisionId if that helps.

@Zache It's not enough. If we tried to check if wgRevisionId == wgStableRevisionId and if so, use wgCurRevisionId for loading, then VE would also open the latest revision on pages like https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&stable=1 or https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&oldid=353208, when the user is intentionally viewing an older revision. This could make it difficult to actually undo unreviewed changes in some cases.

stjn added a comment.EditedApr 17 2018, 3:57 PM

VE would also open the latest revision on pages like https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&stable=1

That’s intended, ‘Edit’ button on &stable=1 on stabilised page is supposed to open latest revision (like ‘Edit source’ already does).

matmarex added a comment.EditedApr 17 2018, 4:02 PM

Hmm, okay, but the &oldid= case still stands.

Zache added a comment.EditedApr 17 2018, 4:15 PM

Currently if you edit the stable version (https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&stable=1 )

; Clicking the source edit
https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&action=edit
Result: editing the latest version (OK)

; Clicking the visual edit
https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&action=edit
Result: editing the old version (FAIL)

Currently if you viewing version (https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&oldid=353208 )
; Clicking the source edit
https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&action=edit&oldid=353208
Result: editing the old version (OK)

; Clicking the visual edit
https://test2.wikipedia.org/w/index.php?title=VE-flaggedrevs&action=edit&oldid=353208
Result: editing the old version (OK)

So rule should be that IF oldid url parameter =="" AND wgRevisionId == wgStableRevisionId THEN use wgCurRevisionId

The current behavior for the default wikitext editor is a bit under-documented (at least in English… maybe you have better documentation in German :) ).

I want to make sure we do the exact same thing in visual editor as we do in the default editor, with all the possible weird FlaggedRevs configurations, so I spent some time to find out where this is actually overridden (I wasn't even sure if this behavior was intentional or just a happy little accident). I tracked it down to a bit of code added in rSVN99614 that references T33489 ("bug 31489"), so at least this is nice, I should be able to reuse that.

Also, on second thought, we can't use wgCurRevisionId, because the article may have been edited since we opened the page. We should be loading the actual latest revision. I have to make sure we do this correctly when not dealing with FlaggedRevs.

Also, on second thought, we can't use wgCurRevisionId, because the article may have been edited since we opened the page. We should be loading the actual latest revision. I have to make sure we do this correctly when not dealing with FlaggedRevs.

I verified that we already do this.

While testing, I also discovered that if you view the latest revision of a page with an 'oldid=…' link (e.g. https://en.wikipedia.org/w/index.php?title=User:Matma_Rex/sandbox&oldid=830391803), the "Edit" link will not include an 'oldid' parameter, so if the article is edited in the meantime and you click it, the new latest revision will be opened for editing. This seems like a weird behavior, but I'd be afraid to change it, since it has probably been this way forever… (the code for this is in Skin::editUrlOptions()). For what it's worth, VisualEditor also replicates this.

Change 430009 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Let other editors know which revision of the page to edit

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

Change 430010 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.DesktopArticleTarget.init: Edit the latest revision when viewing a FlaggedRevs-stable one

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

Change 430009 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Let other editors know which revision of the page to edit

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

Change 430010 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.DesktopArticleTarget.init: Edit the latest revision when viewing a FlaggedRevs-stable one

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

While we have implemented a JavaScript fix in ruwiki for stabilized articles, from time to time we have users (example) who switch somehow (knowingly or unknowingly) to the stable version of not-a-stabilized article. Then they edit with VisualEditor, probably don't notice the warning that they are editing an old version, save the article—only to get an ORES-based bot to revert their edit due to deletion of text. Is this fix going to exclude such cases, or does it apply only to stabilized articles?

@Jack_who_built_the_house I think in this case, both the wikitext editor and the visual editor will edit the version being viewed (stable), rather than the latest one. This has always worked like this, and in these patches I'm only changing the behavior of visual editor to match wikitext editor where it was previously different.

Although I might be wrong, FlaggedRevs has too many configuration options and it's hard to tell what will actually happen on a given wiki… You should be able to test a specific situation on https://ru.wikipedia.beta.wmflabs.org/ (I can give you any user rights there if you tell me your account's username).

The exact behavior is [FlaggablePageView::setViewTabs()]:

If:

  • Page is in a reviewable namespace
  • Page has a stable revision
  • Stable revision is not "synced": [stableVersionIsSynced()]
    • Page has a pending revision
    • OR Transcluded templates or embedded files have been modified since the latest revision was reviewed
  • Stable revision is shown by default [isStableShownByDefault()]:
    • Because this is the behavior for all pages ($wgFlaggedRevsOverride=true)
    • OR Because the specific page has been "stabilized"
  • The user is viewing a stable revision: [showingStable()]
    • Because they requested it: [showingStableByRequest()]
      • User is just viewing the page, rather than history, diff, page info, etc.
      • Using URL parameter 'stable=1', but not 'oldid', 'stableid'
    • OR Because this is the default: [showingStableAsDefault()]
      • User has not overridden the preference to view drafts by default
      • User is not in a group that views drafts by default

Then:

  • Change the "Edit" tab to point to the latest version of the article, rather than the version being viewed
  • Instruct VisualEditor to edit the latest version of the article, rather than the version being viewed (this is the only thing I changed in my patches)

(This is already simplified from the real thing, the real thing is like 100 lines of code spread across a dozen functions, but hopefully I didn't skip anything important; a lot of conditions seem to be checked multiple times)

So, in this case, because the configuration on ruwiki is $wgFlaggedRevsOverride=false and the page is not stabilized, the "Edit" link for both VE and wikitext will point to the revision being viewed (the old stable one).

On dewiki in the same situation, the "Edit" links would point to the latest pending revision, because $wgFlaggedRevsOverride=true (unless the page was "de-stabilized").

matmarex closed this task as Resolved.May 9 2018, 10:19 PM