Page MenuHomePhabricator

Mobile wikitext editing broken when user isn't in Schema:Edit bucket
Closed, ResolvedPublic1 Estimated Story Points

Description

When Schema:Edit logging is enabled but the user isn't in the bucket – 93.75% of users – mobile edits will succeed but the editor overlay won't close. When the user refreshes the page they'll see the "Success! You're edit was saved." toast.

This regression was introduced in 9b239fa.

I suspect that this is the cause of https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Edit_acts_like_it_won.27t_save.

  • wmf.13 hasn't been deployed to enwiki yet and it looks as if it's already been resolved.
    • This code is in wmf.12 as well.

Event Timeline

phuedx claimed this task.
phuedx raised the priority of this task from to High.
phuedx updated the task description. (Show Details)
phuedx added projects: MobileFrontend, Regression.

Change 269751 had a related patch set uploaded (by Phuedx):
Make SchemaEdit#log always return jQuery.Promise

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

This bug is in -wmf.13 so a SWAT today should be organised

phuedx updated the task description. (Show Details)

Change 269755 had a related patch set uploaded (by Alex Monk):
Make SchemaEdit#log always return jQuery.Promise

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

Moving to Ready for Signoff as 269755 has been merged. @Krenair will SWAT deploy 269755. Thanks @Krenair!

Change 269751 merged by jenkins-bot:
Make SchemaEdit#log always return jQuery.Promise

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

Ay, this is my fault; thank you for noticing it, @phuedx, and thank you for deploying the fix, @Krenair.

For my own information, EditorOverlayBase.onSaveComplete is the code that needed the promise to be settled, right?

Change 269848 had a related patch set uploaded (by Jforrester):
Make SchemaEdit#log always return jQuery.Promise

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

Change 269848 merged by Alex Monk:
Make SchemaEdit#log always return jQuery.Promise

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

Change 269755 merged by Alex Monk:
Make SchemaEdit#log always return jQuery.Promise

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

Jdforrester-WMF raised the priority of this task from High to Unbreak Now!.
Jdforrester-WMF edited a custom field.

Now deployed to prod in both wmf.12 and wmf.13 and tested as working. Thanks for the catch, sorry for the problem.

In T126497#2016762, @Neil_P._Quinn_WMF wrote:

Ay, this is my fault; thank you for noticing it, @phuedx, and thank you for deploying the fix, @Krenair.

For my own information, EditorOverlayBase.onSaveComplete is the code that needed the promise to be settled, right?

You're correct.

@Nuria correctly pointed out on 269848 that logging events should be considered fire and forget so returning anything from #log methods doesn't really make sense. I'll be submitting a patch today or tomorrow to address this.