Page MenuHomePhabricator

Some EditAttemptStep events on existing pages do not have revision IDs
Closed, ResolvedPublic

Description

Problem

All EditAttemptStep events that aren't creating a new page (identified by page_id != 0) should have a valid revision_id (the newly saved revision for saveSuccess events, and the base revision for the edit for all the rest).

However, some events don't. This is the count of non-page creating events with a revision ID of 0 during the 90 days ending 4 February 2019.

platformeditor_interfaceabortfirstChangeinitloadedreadysaveAttemptsaveFailuresaveIntentsaveSuccess
desktopvisualeditor53,00028,00071,00063,00063,00022,0005,30021,000220
desktopwikitext820,0001,200,00002,400,0002,400,0000000
desktopwikitext-2017123126212391140
phonevisualeditor13036,000210,000120,000120,00023,0003,50027,000580
phonewikitext67,00021,000470,000400,000400,00017,0001,10023,000280

This appears to be happening across all our editors, but not really affecting saveSuccess events (probably thanks to our recent work on T226847).

The desktop wikitext editor is the most heavily affected. Here's the problematic events as a proportion of all events:

platformeditor_interfaceabortfirstChangeinitloadedreadysaveAttemptsaveFailuresaveIntentsaveSuccess
desktopvisualeditor3.4%12.6%4.3%4.1%4.1%12.0%13.2%11.9%0.1%
desktopwikitext51.4%88.0%0.0%70.1%70.1%0.0%0.0%0.0%0.0%
desktopwikitext-20170.1%0.1%0.1%0.0%0.1%0.0%0.1%0.0%0.0%
phonevisualeditor0.0%4.0%4.3%3.7%3.7%3.5%4.2%3.7%0.1%
phonewikitext1.5%3.6%5.5%4.9%4.9%3.5%3.3%3.2%0.1%

Source notebook.

Event Timeline

A couple of initial notes:

  • This is not blocking any of our planned analysis of VE-as-default or edit cards.
  • I imagine the intention behind logging the ID of the base revision (as opposed to logging the ID of the new revision in saveSuccess events) is to help us pinpoint base revisions that are breaking the editor somehow, but I doubt we've ever actually used it. So maybe we should just stop doing it. @DLynch, @Esanders, any thoughts?

So, null revision_id means that mw.config.get( 'wgRevisionId' ) didn't return anything. This implies bigger issues with the current session...

I just rechecked this, and it's still a problem. I've updated the numbers in the description. @Mayakp.wiki, I put my code in the notebook which is linked in the description.

I think I just realized what is causing this – when you open the old wikitext edit page, mw.config.get( 'wgRevisionId' ) is not set. When you switch to VE and save the page, you get this effect.

Elsewhere in the code, we use $( 'input[name=parentRevId]' ).val() instead, maybe we should do the same here.

edit: This is a known behavior: T62548

Change 571555 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Fix EditAttemptStep events logged with revision_id=0

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

Change 571555 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Fix EditAttemptStep events logged with revision_id=0

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

matmarex removed a project: Patch-For-Review.

That should fix this. We should check if this is still happening for new events after the change is deployed (after 20 February).

Thanks @matmarex , I have opened T244939 to QA this after Feb 20.

ppelberg claimed this task.

@Mayakp.wiki found in March that this is still an issue (T244939#5981084).

Change 651234 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/WikiEditor@master] Edit page normally lacks wgRevisionId, but does have wgCurRevisionId

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

Change 651234 merged by jenkins-bot:
[mediawiki/extensions/WikiEditor@master] Edit page normally lacks wgRevisionId, but has form field parentRevId

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

ppelberg added a subscriber: MNeisler.

Maybe resolved for real this time?

Per today's team meeting, I'm assigning this task over to @MNeisler / Product-Analytics to verify whether https://gerrit.wikimedia.org/r/651234 resolved this issue or not.

LGoto triaged this task as Medium priority.Jan 19 2021, 6:09 PM
LGoto moved this task from Triage to Current Quarter on the Product-Analytics board.
Aklapper subscribed.

@MNeisler: Hi, all related patches in Gerrit have been merged. Can this task be resolved (via Add Action...Change Status in the dropdown menu), or is there more to do in this task? Asking as you are set as task assignee. Thanks in advance!

Per the conversation @MNeisler and I had offline today, we're going to consider this task resolved for two reasons:

  1. This issue has NOT surfaced in any of the analyses we've run that involve EditAttemptStep over the past (almost) 4 years
  2. We seldom use revision_id associated with non-SaveSuccess actions or events which is what this issue potentially impacts (related to "1)")