Page MenuHomePhabricator

ConfirmEdit shouldn't compute links added when just viewing the edit form
Closed, ResolvedPublic

Description

Apparently, per https://gerrit.wikimedia.org/r/#/c/199679/ , ConfirmEdit is attempting to find which links were added when viewing the edit form (action=edit). That patch changes it to do a slave query, but I don't think any query should be necessary.

On action=edit, no links will have been added yet as they're just editing the page (unless it's a preload, but that's relatively rare).

And most importantly (regardless of whether it's a preload) you don't know the final links they're adding (if any) until they save.

(Though it's possible to try to show it on preview speculatively for a better UX along the lines of https://git.wikimedia.org/commit/mediawiki%2Fextensions%2FConfirmEdit/7e0651a3c412a1b35096bc666901c43dcffeaa8a).

Event Timeline

Wouldn't it be enough to limit all edit checks to be made if the actual action is submit? I'm not sure, if a user could submit edits with action edit in some way?

Wouldn't it be enough to limit all edit checks to be made if the actual action is submit? I'm not sure, if a user could submit edits with action edit in some way?

Yes, I think that should work. As far as I know, action=edit never does a write to page content.

Ok, thanks for info @Mattflaschen! Let's doing this :)

@Mattflaschen I looked into the code, and I haven't found any condition where the addurl trigger would make a db query, if the edit form isn't submitted? :/ Can you point me to the code you mean? :)

Sorry, I don't know. But if it doesn't do it on edit views/GET requests, I don't know why https://gerrit.wikimedia.org/r/#/c/199679/ was necessary (that's what triggered me to post this).

I believe there was also some content handler change to fix compatibility with core that introduced a regression like this and was later fixed. So it may have been combining with the master usage of that method to trigger log warnings...possibly.

Sorry, my fault. you're right, the db query is triggered on edit actions, too (especially, if $wgCaptchaRegexes is set. I'll upload a fix for it :)

Change 230960 had a related patch set uploaded (by Florianschmidtwelzow):
Don't check for edits that will not be saved

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

Change 231255 had a related patch set uploaded (by Florianschmidtwelzow):
Follow up for change in ConfirmEdit

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

Change 230960 merged by jenkins-bot:
Don't check for edits that will not be saved

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

Change 231255 merged by jenkins-bot:
Follow up for change in ConfirmEdit

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