Page MenuHomePhabricator

Prevent edits from anonymous users with add link plugin
Open, HighPublicBUG REPORT

Description

If a user is logged in, begins an Add-Link task, and then is logged out (via another tab, session expiration, whatever) they'll see something like this:

image.png (624×1 px, 279 KB)

and that generates history / logs like this:

image.png (70×1 px, 84 KB)

image.png (60×1 px, 71 KB)

In onVisualEditorApiVisualEditorEditPreSave(), after we have verified that we have data from the link recommendation plugin, we should check that the user is logged in. If the user is not logged in, we should return an error message. The error message should be: "We could not save your edit because you are no longer logged in. Please log in and try again.", with "log in" linking to Special:UserLogin.

It is okay if the user loses their in-progress edit when this happens, since this case will be rare.

Event Timeline

kostajh triaged this task as Medium priority.Sep 14 2021, 12:53 PM
kostajh created this task.
kostajh changed the subtype of this task from "Task" to "Bug Report".Sep 14 2021, 12:53 PM
kostajh added subscribers: MMiller_WMF, RHo.

@RHo @MMiller_WMF could you please provide wording that should be used with the error message?

@RHo @MMiller_WMF could you please provide wording that should be used with the error message?

Hi @kostajh - based on the task description, is it the case that the attempted 'Add link' edit on Article X will be lost after they log in? Or is it possible to enable them to keep the edits after logging in? I might be a little frustrated as the person who worked on the article to have the edit lost and essentially be unable to work on that specific article again due to the current raondomised reloading of tasks.

@RHo @MMiller_WMF could you please provide wording that should be used with the error message?

Hi @kostajh - based on the task description, is it the case that the attempted 'Add link' edit on Article X will be lost after they log in? Or is it possible to enable them to keep the edits after logging in? I might be a little frustrated as the person who worked on the article to have the edit lost and essentially be unable to work on that specific article again due to the current raondomised reloading of tasks.

Yeah, good point.

It's possible to keep the edit, but the full workflow looks like this:

  • start an add link task
  • get logged out due to session expiration or whatever
  • get an error message from VisualEditor saying you're not logged in anymore
  • press "Try again"
  • get an error message from GrowthExperiments saying that you should log in
  • open a new browser tab and login
  • press "Try again"
  • get an error message from VisualEditor, this time saying that you were anonymous, but now you're logged in as user X, do you want to make this edit?
  • press "Try again"
  • success

so... not great. We could probably suppress the VisualEditor errors but we're still looking at a workflow that involves logging in via another tab, which will be awkward.

@RHo @MMiller_WMF could you please provide wording that should be used with the error message?

Hi @kostajh - based on the task description, is it the case that the attempted 'Add link' edit on Article X will be lost after they log in? Or is it possible to enable them to keep the edits after logging in? I might be a little frustrated as the person who worked on the article to have the edit lost and essentially be unable to work on that specific article again due to the current randomised reloading of tasks.

Yeah, good point.

It's possible to keep the edit, but the full workflow looks like this:

  • start an add link task
  • get logged out due to session expiration or whatever
  • get an error message from VisualEditor saying you're not logged in anymore
  • press "Try again"
  • get an error message from GrowthExperiments saying that you should log in
  • open a new browser tab and login
  • press "Try again"
  • get an error message from VisualEditor, this time saying that you were anonymous, but now you're logged in as user X, do you want to make this edit?
  • press "Try again"
  • success

so... not great. We could probably suppress the VisualEditor errors but we're still looking at a workflow that involves logging in via another tab, which will be awkward.

OK, given this should hopefully not happen too often(?), it makes sense and easier approach to provide an error message per the task description that lets the user know the edit is lost because they were logged out and they need to login to access Add links tasks again.
Hopefully if we do T279025 in the future, users who experience this could at least return to try the same edit again.

How about the following for the error message copy?

We could not save your edit because you are no longer logged in. Please log in and try again.
[Log in]

MMiller_WMF raised the priority of this task from Medium to High.Sep 21 2021, 2:19 AM

Hi @RHo @MMiller_WMF I wanted to confirm which kind of dialog should be shown along with the provided message.

Should the text be shown in the red box like the existing error message

Screen Shot 2021-09-28 at 10.20.18 AM.png (728×1 px, 135 KB)

or should it be in the dialog similar to the one used when another user edited the article?

Screen Shot 2021-09-28 at 10.22.01 AM.png (310×630 px, 41 KB)

Should "log in" be a button or is it just a link within the message itself?

Hi @RHo @MMiller_WMF I wanted to confirm which kind of dialog should be shown along with the provided message.

Should the text be shown in the red box like the existing error message

Screen Shot 2021-09-28 at 10.20.18 AM.png (728×1 px, 135 KB)

or should it be in the dialog similar to the one used when another user edited the article?

Screen Shot 2021-09-28 at 10.22.01 AM.png (310×630 px, 41 KB)

Should "log in" be a button or is it just a link within the message itself?

The second one definitely looks better, but I wonder if it's worth addinf a cancel to give the user the option to return to the edit summary and review the details, in the off chance they want to retry the edit without suggested edits mode?

In that case, should the dialog appear on top of the edit summary dialog so that when the user cancels, they still see the edit summary dialog?

Screen Shot 2021-09-28 at 11.28.24 AM.png (750×1 px, 165 KB)

In that case, should the dialog appear on top of the edit summary dialog so that when the user cancels, they still see the edit summary dialog?

Screen Shot 2021-09-28 at 11.28.24 AM.png (750×1 px, 165 KB)

Yes that's true. And in that case and given it is an error message after all, let's use the existing error message with the red border, with the button below the error message as "Login" instead of "Try again". Would that work?

Change 725107 had a related patch set uploaded (by MewOphaswongse; author: MewOphaswongse):

[mediawiki/extensions/GrowthExperiments@master] Add a link: Prevent edits from anonymous users

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

Hi @RHo — that does work (albeit without some hacks since the error pane & buttons are from standard OOUI ProcessDialog so the implementation had to override the default). Here are the updated error screens:

Mobile experience:

addlnk_logout.gif (1×750 px, 405 KB)

Desktop experience:

addlnk_logout_dt.gif (1×1 px, 1 MB)

Thanks @mewoph LGTM (but also hope it's an error that will be rarely seen)

@RHo @MMiller_WMF @nettrom_WMF would you like instrumentation for any of the following:

  • The error dialog being shown to the user
  • Clicking Dismiss
  • Clicking Login
  • When the user arrives back on Special:Homepage, do you want the query parameter to show that the user arrived from this workflow (in the same way that we track the user arrived from WelcomeSurvey, Personal tools link, etc)

As this particular scenario seems pretty rare, it is maybe OK to skip instrumentation, but thought I'd ask.

@RHo @MMiller_WMF @nettrom_WMF would you like instrumentation for any of the following:

  • The error dialog being shown to the user
  • Clicking Dismiss
  • Clicking Login
  • When the user arrives back on Special:Homepage, do you want the query parameter to show that the user arrived from this workflow (in the same way that we track the user arrived from WelcomeSurvey, Personal tools link, etc)

As this particular scenario seems pretty rare, it is maybe OK to skip instrumentation, but thought I'd ask.

Thanks @kostajh - agree this doesn't seem worth instrumenting.

As this particular scenario seems pretty rare, it is maybe OK to skip instrumentation, but thought I'd ask.

I also agree with that assessment, I don't think this needs to be instrumented.

One clarifying question: does the click on "Submit" trigger an event in EditAttemptStep with action="saveAttempt", or are we overriding everything and handling it on our end? If it gets logged in EditAttemptStep, and we're at some point start getting concerned about this (e.g. because of a decrease in saved edits or community members run into it), then we can identify it that way.

I agree about not instrumenting. Thank you!

As this particular scenario seems pretty rare, it is maybe OK to skip instrumentation, but thought I'd ask.

I also agree with that assessment, I don't think this needs to be instrumented.

One clarifying question: does the click on "Submit" trigger an event in EditAttemptStep with action="saveAttempt", or are we overriding everything and handling it on our end? If it gets logged in EditAttemptStep, and we're at some point start getting concerned about this (e.g. because of a decrease in saved edits or community members run into it), then we can identify it that way.

Yes, this does trigger dialog-save/saveAttempt/safeFailure events:

{"event":{"feature":"mwSave","action":"dialog-save","editingSessionId":"268af10def3c76db968c","is_oversample":false,"user_id":0,"user_editcount":362,"editor_interface":"visualeditor","integration":"page","platform":"desktop"},"schema":"VisualEditorFeatureUse","webHost":"localhost","wiki":"my_wiki","revision":-1};
"event":{"version":1,"action":"saveAttempt","is_oversample":false,"editor_interface":"visualeditor","integration":"page","page_id":1016,"page_title":"Douglas_Adams","page_ns":0,"revision_id":1971,"editing_session_id":"268af10def3c76db968c","page_token":"8c209d54aa441b82bd25","session_token":"b0fa67a33f81a9f137d3","user_id":0,"user_editcount":362,"mw_version":"1.38.0-alpha","platform":"desktop","user_class":"IP","save_attempt_timing":50774},"schema":"EditAttemptStep","webHost":"localhost","wiki":"my_wiki","revision":-1};=
{"event":{"version":1,"action":"saveFailure","is_oversample":false,"editor_interface":"visualeditor","integration":"page","page_id":1016,"page_title":"Douglas_Adams","page_ns":0,"revision_id":1971,"editing_session_id":"268af10def3c76db968c","page_token":"8c209d54aa441b82bd25","session_token":"b0fa67a33f81a9f137d3","user_id":0,"user_editcount":362,"mw_version":"1.38.0-alpha","platform":"desktop","user_class":"IP","save_failure_type":"responseUnknown","save_failure_timing":1482,"save_failure_message":"hookaborted"},"schema":"EditAttemptStep","webHost":"localhost","wiki":"my_wiki","revision":-1};

Change 725107 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Add a link: Prevent edits from anonymous users

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