Page MenuHomePhabricator

Mobile wikitext editing broken when user isn't in Schema:Edit bucket
Closed, ResolvedPublic1 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.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : wmf/1.27.0-wmf.13Make SchemaEdit#log always return jQuery.Promise
mediawiki/extensions/MobileFrontend : wmf/1.27.0-wmf.12Make SchemaEdit#log always return jQuery.Promise
mediawiki/extensions/MobileFrontend : masterMake SchemaEdit#log always return jQuery.Promise

Event Timeline

phuedx created this task.Feb 10 2016, 7:18 PM
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.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 10 2016, 7:18 PM

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

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

phuedx updated the task description. (Show Details)Feb 10 2016, 7:23 PM

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

phuedx updated the task description. (Show Details)Feb 10 2016, 7:31 PM
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

phuedx added a subscriber: Krenair.Feb 10 2016, 7:37 PM

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?

Jdforrester-WMF updated the task description. (Show Details)

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 closed this task as Resolved.Feb 10 2016, 10:47 PM
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.

phuedx added a subscriber: Nuria.Feb 11 2016, 4:09 AM

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.