MobileFrontend preserves the mfnotify=saved query parameter in the "Desktop" URL, which triggers the edit association dialog. mfnotify is used to signal that a mobile edit just occurred, and MF itself removes it from the URL upon page load. I think MF should remove it from the desktop URL.

Reader Growth will triage this in the next week or so. This may originate from a part of Mobile Frontend that the Editing team primarily maintains; if so we'll loop them in.

In T408267#11314356, @Daimona wrote:

MobileFrontend preserves the mfnotify=saved query parameter in the "Desktop" URL, which triggers the edit association dialog. mfnotify is used to signal that a mobile edit just occurred, and MF itself removes it from the URL upon page load. I think MF should remove it from the desktop URL.

Sounds like the fix here will involve changing the code that generates the "view on desktop" URL at the bottom of the page to ensure that the mnotify=saved param is removed if it would otherwise be present? Are there any other "mobile only" URL params that may need to be removed here?

Specifically, it looks like we might want to call $req->unsetVal( 'mfnotify' ) in the body of the getDesktopViewLink method in MobileFrontendSkinHooks.php.

Hi @vaughnwalters / @Daimona / @ifried – the Reader Growth team can pick this up for estimation and inclusion into a sprint in the near future. I suspect that the fix here will involve changing the way the "view on destop" link in the page footer is constructed in PHP – we probably want to strip out certain URL params if they are present. However, many of the engineers on the team (including myself) are new to MobileFrontend. It would be helpful to have some more detailed context and repro instructions before we begin work here. In particular I'd like to request the following:

• Repro environment – is the screen recording in the task description coming from a local Wiki instance, or from Beta, or from a production Wiki? If local, what extensions / config are you running that may be relvant? If on beta or prod, a link to a testable page would be great.
• More detailed repro instructions – what is meant by "associate the edit with an event"? I'm not familiar with this workflow. Is it specific to a particular extension? If so, how do I enable it and set it up locally? How exacty are you "switching to mobile skin" – by clicking the link in the footer, or by doing something else? Please provide all necessary context.

Please update the task description with this information. Once that happens, we can pick this up to work on it in the near future. Thanks!

In T408267#11351216, @egardner wrote:

In T408267#11314356, @Daimona wrote:

MobileFrontend preserves the mfnotify=saved query parameter in the "Desktop" URL, which triggers the edit association dialog. mfnotify is used to signal that a mobile edit just occurred, and MF itself removes it from the URL upon page load. I think MF should remove it from the desktop URL.

Sounds like the fix here will involve changing the code that generates the "view on desktop" URL at the bottom of the page to ensure that the mnotify=saved param is removed if it would otherwise be present?

Yeah, I think that'd be the simplest fix.

Are there any other "mobile only" URL params that may need to be removed here?

No clue about this :-/

In T408267#11351304, @egardner wrote:

It would be helpful to have some more detailed context and repro instructions before we begin work here.

I'll let Vaughn answer about the screencast, but I think you don't need that to reproduce the underlying issue. You could make an edit in mobile (to any page), then click to switch to desktop, and see how the mfnotify parameter remains in the desktop URL.

Note that @vaughnwalters is out until Friday this week, but he can respond with more details when he's back.

@vaughnwalters when you're back, can you give me a little more information about how to reproduce the scenario where you encountered this bug? The "associate an edit with an event" part is what I don't really understand – I assume this is a CampaignEvents feature. If there's a dummy campaign I can use on Beta or something (or if it's easy to set up something in a local environment) then I can try to reproduce this bug to confirm that my proposed fix will work.

Should be a quick fix after that.

Change #1204976 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/extensions/MobileFrontend@master] Remove mvnotify params when constructing desktop link

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

In T408267#11372910, @egardner wrote:

If there's a dummy campaign I can use on Beta or something (or if it's easy to set up something in a local environment) then I can try to reproduce this bug to confirm that my proposed fix will work.

FWIW, this should now be reproducible in beta. The steps would be:

• Visit Special:RegisterForEvent/1246
• Click "Register"
• Open a random article
• Click "Mobile view" in the footer to go to the mobile version
• Make an edit
• Select either option in the dialog that shows up
• Click "Desktop" in the footer

However... I just tested it and it doesn't reproduce. I'm pretty sure I had managed to do it locally, so not sure what's going on...

In T408267#11372987, @Daimona wrote:

In T408267#11372910, @egardner wrote:

If there's a dummy campaign I can use on Beta or something (or if it's easy to set up something in a local environment) then I can try to reproduce this bug to confirm that my proposed fix will work.

FWIW, this should now be reproducible in beta. The steps would be:

• Visit Special:RegisterForEvent/1246
• Click "Register"
• Open a random article
• Click "Mobile view" in the footer to go to the mobile version
• Make an edit
• Select either option in the dialog that shows up
• Click "Desktop" in the footer

However... I just tested it and it doesn't reproduce. I'm pretty sure I had managed to do it locally, so not sure what's going on...

Thanks @Daimona – I just went through these steps a few times (editing on Desktop and switching to Mobile, and then vice-versa). I can see the initial "associate an edit" dialog but not any duplicate one.

The only way I was able to trigger the duplicate dialogs was by manually adding mfnotify=save to my URL and re-loading the page. In that case the dialog appears even without any edit on my part, and clicking the view on desktop link in the footer (which preserves the mfnotify param) triggers the dialog again. But I'd say this is expected behavior since I added this to the URL by hand.

I do have a basic patch here to remove this param from that link programmatically, but I'd like to understand how this bug can be reproduced in a more organic way before merging.

Yeah, it's odd because I just tried this locally and it reproduces the issue, with the exact same steps. Not sure why beta is different though...

In T408267#11373031, @Daimona wrote:

Yeah, it's odd because I just tried this locally and it reproduces the issue, with the exact same steps. Not sure why beta is different though...

Can you pull down https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1204976 and see if that fixes things in your local env? If so I'm happy to move it forward.

In T408267#11373059, @egardner wrote:

In T408267#11373031, @Daimona wrote:

Yeah, it's odd because I just tried this locally and it reproduces the issue, with the exact same steps. Not sure why beta is different though...

Can you pull down https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1204976 and see if that fixes things in your local env? If so I'm happy to move it forward.

Yep, it seems to fix it for me. I'll let @vaughnwalters do more testing and someone more familiar with MF approve, but the fix makes sense to me.

In T408267#11374379, @Daimona wrote:

In T408267#11373059, @egardner wrote:

In T408267#11373031, @Daimona wrote:

Yeah, it's odd because I just tried this locally and it reproduces the issue, with the exact same steps. Not sure why beta is different though...

Can you pull down https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1204976 and see if that fixes things in your local env? If so I'm happy to move it forward.

Yep, it seems to fix it for me. I'll let @vaughnwalters do more testing and someone more familiar with MF approve, but the fix makes sense to me.

Tested with that patch, and it fixes it for me on local as well.

Change #1204976 merged by Eric Gardner:

[mediawiki/extensions/MobileFrontend@master] Remove mfnotify params when constructing desktop link

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

Hey @vaughnwalters, now that this patch has merged, can you verify that it fixes the issue on production when the train rolls out? Please ping me if so and I'll close this out. Thanks!

Added to Connection team QA column so that @vaughnwalters can take a look.

Thank you so much for your work on this, @egardner!

Working as expected now in prod, thx @egardner! This can be closed out now