Page MenuHomePhabricator

Change what event and dialog is shown when people are prevented from switching
Open, MediumPublic

Description

This task is about revising the event that is fired when someone attempts to switch from the Reply Tool's source to visual mode after drafting a comment that contains any combination of the following syntax "types":

  • Extension
  • Table
  • Template

Implementation details

When someone does the following:

  1. Writes a comment in the tool's source mode that contains any combination of a table, template or extension
  2. Attempts to switch to the tool's visual mode
  3. The following event should fire:
Event nameSchema
dialog-prevent-showVisualEditorFeatureUse
  1. They should see a dialog that says:
Dialog titleDialog body
Visual mode disabledSorry, switching to visual mode is disabled because <b>$1</b> was detected in the comment you have written. [https:\/\/www.mediawiki.org\/wiki\/Special:MyLanguage\/Extension:DiscussionTools/Reply_tool_visual_mode_limitations Learn more].

Where $1 = "table," "template," or "extension."

Open questions

  • Is it possible to implement the dialog messaging described in "Step 4." above without firing logging/firing separate events for each syntax type?

Yes. See: T259673#6428022.

Done

Event Timeline

ppelberg created this task.Aug 4 2020, 11:53 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 4 2020, 11:53 PM

I'd recommend dialog-prevent-table-template-show. If we wanted to simplify, something like dialog-prevent-both-show would also work but that might cause confusion if there are ever any additional events beyond template and table that might cause this dialog to show.

Let me know if you have any questions or concerns.

cc @ppelberg @DLynch

I'd recommend dialog-prevent-table-template-show. If we wanted to simplify, something like dialog-prevent-both-show would also work but that might cause confusion if there are ever any additional events beyond template and table that might cause this dialog to show.

Let me know if you have any questions or concerns.

cc @ppelberg @DLynch

Thank you, Megan.

As discussed during today's standup, I'm going to assign this over to @Esanders to share any input on the event's name as David is OoO this week.

There are three syntax types we block on:

  • Templates
  • Tables
  • Extensions (including references)

This makes the logging and messaging more complex. Currently we just fail on the first one we find. Displaying/reporting all the various combinations would be more complex, and it seems like it would be relatively rare.

This makes the logging and messaging more complex. Currently we just fail on the first one we find. Displaying/reporting all the various combinations would be more complex, and it seems like it would be relatively rare.

...in light of what Ed shard above and the subsequent conversation @Mayakp.wiki, @MNeisler and I had, we're going to make the changes described below.

Changes

  1. Log 1 event, regardless of the syntax type that causes a switch from being blocked
  2. Call this "event" dialog-prevent-show
  3. This event should be logged in the following schema: VisualEditorFeatureUse

Side effects
In implementing the "Changes" above, we will be "undoing" the events [i] implemented in T257501.

Rationale
We are okay with not knowing which specific syntax (table, template or extension) is causing people to be blocked from switching text input modes because we are primarily interested in knowing WHEN someone is blocked from switching because the new syntax will effect tables, templates and extensions equally.

Note: the task title and description has been updated to reflected the new scope defined in this comment.


i.

EventEvent nameSchema
Person tries to switch to the Reply Tool's visual mode after writing a template in its source modedialog-prevent-template-showVisualEditorFeatureUse
Person tries to switch to the Reply Tool's visual mode after writing a table in its source modedialog-prevent-table-showVisualEditorFeatureUse
ppelberg renamed this task from Add dialog and event when people attempt to switch from source to visual with a template and table to Change what event and dialog is shown when people are prevented from switching.Aug 13 2020, 11:58 PM
ppelberg updated the task description. (Show Details)
ppelberg updated the task description. (Show Details)

Open question

  • @Esanders is it possible to implement the dialog message described in "Step 4." of the task description's "Implementation details" section [i] without logging/firing separate events for each syntax type?

i.

  1. They should see a dialog that says:
Dialog titleDialog body
Visual mode disabledSorry, switching to visual mode is disabled because the syntax below was detected in the comment you have written. [Learn more](mw:Extension:DiscussionTools/Reply_tool_visual_mode_limitations). <ul> <li>Extension</li> <li>Table</li> <li>Template</li> </ul>
MNeisler moved this task from Triage to Tracking on the Product-Analytics board.

Open question

  • @Esanders is it possible to implement the dialog message described in "Step 4." of the task description's "Implementation details" section [i] without logging/firing separate events for each syntax type?

@Esanders confirmed during today's standup that it is possible to implement the dialog message described in "Step 4." of the task description's "Implementation details" section] without logging/firing separate events for each syntax type?

The task description's "Open questions" section has been updated to reflect the above.

ppelberg updated the task description. (Show Details)Sep 1 2020, 7:49 PM
ppelberg updated the task description. (Show Details)Sep 2 2020, 12:03 AM
ppelberg reassigned this task from Esanders to DLynch.Sep 14 2020, 5:12 PM
ppelberg updated the task description. (Show Details)Sep 16 2020, 7:33 PM
ppelberg moved this task from Inbox to High Priority on the Editing QA board.Sep 17 2020, 9:43 PM

Change 628204 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/DiscussionTools@master] When preventing switching modes, don't log a type-specific action

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

Change 628204 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] When preventing switching modes, don't log a type-specific action

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

The above patch unifies the logging to one type. I think changing the message is lower priority as it will be quite rare for more than one disallowed syntax type to be present.

The above patch unifies the logging to one type. I think changing the message is lower priority as it will be quite rare for more than one disallowed syntax type to be present.

Roger that.

Meta: is there something about the revisions to the error message implementation that contributed to you thinking it was best handled in a separate task? E.g. did it prompt questions we do not yet have answers for that you thought would get in the way of consolidating the two events?

ppelberg updated the task description. (Show Details)Sep 23 2020, 9:50 PM

(I've updated the task description to reflect what copy is shown in the resulting dialog.)

Ryasmeen edited projects, added Verified; removed Editing QA.Sep 23 2020, 10:27 PM
ppelberg reassigned this task from DLynch to MNeisler.Sep 25 2020, 1:12 AM
ppelberg updated the task description. (Show Details)

I'm assigning this over to @MNeisler now that we've verified the correct event is being emitted.

MNeisler triaged this task as Medium priority.Sep 25 2020, 5:07 PM

Quick update re post-deployment data QA: I'm currently only seeing two dialog-prevent-show events recorded in VisualEditorFeatureUse since the beginning of September through yesterday. Both events were recorded on Dutch Wikipedia on October 13th. Based on the sample event from @Ryasmeen, I'm guessing both of these might be testing events. See below:

It might be possible that no users have performed the action required to cause this event since deployment but it is difficult to confirm or complete QA until some more events are fired. I tried creating a few more events on a few different wikis (frwiki, cswiki, and kowiki) and will confirm if they are logged correctly tomorrow once they are stored in eventlogging.

For the two events recorded, everything looks as expected except for the editor interface. The editor_interface for these events are recorded as visual editor - @DLynch Is that expected? I had assumed that the editing interface should be wikitext given that this action is supposed to record when an editor is prevented from switching to visual mode from source?

MNeisler updated the task description. (Show Details)Oct 14 2020, 6:15 PM

Quick update re post-deployment data QA: I'm currently only seeing two dialog-prevent-show events recorded in VisualEditorFeatureUse since the beginning of September through yesterday. Both events were recorded on Dutch Wikipedia on October 13th. Based on the sample event from @Ryasmeen, I'm guessing both of these might be testing events. See below:

It might be possible that no users have performed the action required to cause this event since deployment but it is difficult to confirm or complete QA until some more events are fired. I tried creating a few more events on a few different wikis (frwiki, cswiki, and kowiki) and will confirm if they are logged correctly tomorrow once they are stored in eventlogging.

Update: I've rechecked the data to see if any additional events have come in. I see one event for frwiki, which is likely from my test instance and one for arwiki that occurred on October 16th. See updated numbers below.

JTannerWMF reassigned this task from MNeisler to DLynch.Oct 21 2020, 5:43 PM
JTannerWMF added a subscriber: JTannerWMF.

David will take a look

Change 637110 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/DiscussionTools@master] Switching editor modes would switch editor_interface before success/fail

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

I took a look. There was a sequencing issue with the events -- the editor-switch.visual-desktop event fired as soon as the switch was requested, so if it later failed because of a template the dialog-prevent-show would believe the switch had already happened and log itself as happening in visual mode.

The patch adjusts it so that the editor-switch.[visual/source]-desktop event won't fire until the switch has succeeded, and so the dialog event won't be confused.

Everything else about the event seems to be correct -- it fires whenever the dialog shows, and is easily reproducible by adding e.g. {{done}} to whatever source one is editing.

Change 637110 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Switching editor modes would switch editor_interface before success/fail

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

DLynch reassigned this task from DLynch to MNeisler.Wed, Nov 4, 6:24 PM

I guess you should take a look and verify that it looks correct to you now?

@DLynch - I checked today. There have only been a few events logged since the patch was deployed but the editor_interface for these events appears to still be recorded as visual editor in the VEFU schema.

wikieditor_interfaceplatformfeatureintegrationactionevents
2020-11-09nlwikivisualeditordesktopeditor-switchdiscussiontoolsdialog-prevent-show2
2020-11-03arwikivisualeditordesktopeditor-switchdiscussiontoolsdialog-prevent-show1
2020-10-30plwikivisualeditordesktopeditor-switchdiscussiontoolsdialog-prevent-show1

Data via:

SELECT
    date_format(dt, 'yyyy-MM-dd') AS date,
    wiki AS wiki,
    event.editor_interface AS editor_interface,
    event.platform AS platform,
    event.feature As feature,
    event.integration AS integration,
    event.action AS action,
    COUNT(*) AS events
FROM event.visualeditorfeatureuse
WHERE 
    year = 2020 
    AND month >= 10
    AND wiki <> 'testwiki'
    AND event.action = 'dialog-prevent-show'
GROUP BY
    date_format(dt, 'yyyy-MM-dd'),
    wiki,
    event.editor_interface,
    event.feature,
    event.integration,
    event.platform,
    event.action
DLynch added a comment.Mon, Nov 9, 9:36 PM

@MNeisler Sorry, I didn't realize when I was kicking it over to you -- the general tumult of last week meant we had a disruption to our normal deployment schedule. 1.36.0-wmf.16 only made it to Group 2 wikis in the noon PST deploy today, so your testing was unfortunately not including the fix. (I had foolishly assumed that it'd already be on its way out when I was making that last comment.)

MNeisler reassigned this task from MNeisler to ppelberg.Wed, Nov 11, 2:56 PM

@MNeisler Sorry, I didn't realize when I was kicking it over to you -- the general tumult of last week meant we had a disruption to our normal deployment schedule. 1.36.0-wmf.16 only made it to Group 2 wikis in the noon PST deploy today, so your testing was unfortunately not including the fix. (I had foolishly assumed that it'd already be on its way out when I was making that last comment.)

@DLynch - No problem. Thanks for clarifying. I checked today and confirmed that the editor_interface for these events are now correctly recorded as wikitext in the VEFU schema. See sample event below.

datewikieditor_interfaceplatformfeatureintegrationactionevents
2020-11-10nlwikiwikitextdesktopeditor-switchdiscussiontoolsdialog-prevent-show1

@ppelberg - Data is now appearing as expected so passing to you for sign-off. One note: There are still very few events being logged so far (only 9 total dialog-prevent-show events recorded since deployment of this instrumentation) and many of these are likely test cases so it appears that so far this dialog is rarely encountered; however, this might change as the reply tool is more widely implemented across wikis.

MNeisler updated the task description. (Show Details)
MNeisler updated the task description. (Show Details)Wed, Nov 11, 3:03 PM