Page MenuHomePhabricator

ACW should intercept page creation via VisualEditor
Closed, ResolvedPublic8 Estimated Story Points

Description

ArticleCreationWorkflow doesn't successfully intercept users who are using VisualEditor.

Event Timeline

kaldari set the point value for this task to 8.

Things to note:

  • VE doesn't have any PHP hooks
  • VE creates an 'Edit'/'Create' tab using SkinTemplateNavigation hook and from that point everything is in JS.

Our options:

  1. Have a JS script that changes the href of the 'Create' tab button depending if our conditions are fulfilled
  2. Try to cut it off before user lands on the page, that is basically choose one of the options listed in T173781#3563343

This task is blocked on T173781#3564039 and the subsequent changes to be made to the codebase. If we're going that way then this task can be closed.

This task is blocked on T173781#3564039 and the subsequent changes to be made to the codebase. If we're going that way then this task can be closed.

This turned out to not be quite correct since we still require users to click 'create'/'create source' in order to redirect them to ACW. We still need to handle this in VE.

@Jdforrester-WMF: Any suggestion what would be the best method of intercepting non-autoconfirmed VE users who are trying to create a new article and redirecting them to a different page instead? The options seem to be:

  1. Disable VE when non-autoconfirmed users visit pages that don't exist yet
  2. Override the 'create'/'create source' tab with JS
  3. ?

Yes, if you over-ride the create tab in PHP to not have the ca-edit ID, that'll disable VE.

We can just locally disable VE on eligible pages for eligible users.

We can just locally disable VE on eligible pages for eligible users.

What does "locally disable" mean?

For that particular page.

How is that different from T173605#3582165?

I mean there should be a variable for that.

@Niharika: I think Max's solution is the same as the one suggested by James, he's just elaborating on it. If a user meeting the intercept conditions visits a page that doesn't exist, we just strip the the ca-edit ID from the Create tab, thus preventing VE from utilizing it. Hopefully we can use the SkinTemplateNavigation hook (or something similar) to handle this.

Hmm, looks like the SkinTemplateNavigation hook lets you change the classes, but not the ID :(

The SkinTemplateOutputPageBeforeExec hook should let you modify the ID via the hook's $template parameter. The $template is a SkinTemplate or VectorTemplate object that contains all the data for building the interface. You'll want to look for the content_navigation data specifically (I think you can ignore the content_actions data as I don't think that's used any more). See SkinTemplate::buildContentNavigationUrls() for how the tabs are actually constructed.

@kaldari Max pointed out yesterday that we still won't be able to stop people who click on redlinks and it lands on VE using ?veaction=edit or something.

Redlinks link to title=XXXX&action=edit&redlink=1, so that's what we need to intercept. Basically, we need to intercept any case where action=edit, which AlternateEdit hook is supposed to do, but it seems that VisualEditor is interfering with this somehow.

So it looks like VisualEditor uses the CustomEditor hook to do it's intercepting (which fires before AlternateEdit). We should try using CustomEditor instead of AlternateEdit and see if that fixes some or all of the cases.

P.S. - We'll also have to make sure that wfLoadExtension( 'ArticleCreationWorkflow' ); happens before wfLoadExtension( 'VisualEditor' );.

P.S. - We'll also have to make sure that wfLoadExtension( 'ArticleCreationWorkflow' ); happens before wfLoadExtension( 'VisualEditor' );.

Is that even doable?

Is that even doable?

Yes, with a scary comment.

Change 376588 had a related patch set uploaded (by Niharika29; owner: Niharika Kohli):
[mediawiki/extensions/ArticleCreationWorkflow@master] Skip the landing page for redlinks completely and take user to AfC

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

Change 376588 merged by jenkins-bot:
[mediawiki/extensions/ArticleCreationWorkflow@master] Skip the landing page for redlinks completely and take user to AfC

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

@Niharika, @MaxSem: Niharika's patch solves the "veaction=edit" case, but still doesn't solve the "following a redlink" case. We need to switch the AlternateEdit hook to CustomEditor for that case.

@Niharika, @MaxSem: Niharika's patch solves the "veaction=edit" case, but still doesn't solve the "following a redlink" case. We need to switch the AlternateEdit hook to CustomEditor for that case.

It takes care of both for me.

Change 376638 had a related patch set uploaded (by Niharika29; owner: Niharika Kohli):
[mediawiki/extensions/ArticleCreationWorkflow@wmf/1.30.0-wmf.17] Skip the landing page for redlinks completely and take user to AfC

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

Change 376638 merged by jenkins-bot:
[mediawiki/extensions/ArticleCreationWorkflow@wmf/1.30.0-wmf.17] Skip the landing page for redlinks completely and take user to AfC

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

Doesn't work for me if I set my default editor to VE and then follow a redlink (leading to a URL like https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Nada&action=edit&redlink=1).

@Niharika: I'm not convinced that redirecting users directly to the Article wizard via the ShowMissingArticle hook is the best solution we can come up with for dealing with veaction=edit and veaction=editsource. This is going to be confusing for users who are just looking for articles that don't exist (or even mistype a URL). Perhaps instead we could use the ShowMissingArticle hook to just redirect to the same URL without the veaction parameter (or with the action=edit parameter instead), thus preventing VE from loading.

@Niharika: I'm not convinced that redirecting users directly to the Article wizard via the ShowMissingArticle hook is the best solution we can come up with for dealing with veaction=edit and veaction=editsource. This is going to be confusing for users who are just looking for articles that don't exist (or even mistype a URL). Perhaps instead we could use the ShowMissingArticle hook to just redirect to the same URL without the veaction parameter (or with the action=edit parameter instead), thus preventing VE from loading.

There will still be the 'create' tab that VE creates. That's the thing we're getting around by having it like this.

Change 376773 had a related patch set uploaded (by Kaldari; owner: Kaldari):
[mediawiki/extensions/ArticleCreationWorkflow@master] Switch from AlternateEdit hook to CustomEditor hook

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

Change 376773 merged by jenkins-bot:
[mediawiki/extensions/ArticleCreationWorkflow@master] Switch from AlternateEdit hook to CustomEditor hook

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

MaxSem reassigned this task from Niharika to kaldari.
MaxSem removed a project: Patch-For-Review.