Page MenuHomePhabricator

Options request is made every time a diff is loaded with VE enabled
Closed, ResolvedPublicBUG REPORT

Description

I'm not 100% sure it's a bug, but it seems a weird, counterintuitive behavior to me.

List of steps to reproduce:

What happens?:
An api.php request appears with payload like this:

image.png (88×278 px, 3 KB)

What should have happened instead?:
No request should be made, unless the value of the diffmode query parameter differs from mw.user.options.get( 'visualeditor-diffmode-historical' ) (see below)?

What happens in the code?
See https://phabricator.wikimedia.org/diffusion/EVED/browse/master/modules/ve-mw/preinit/ve.init.mw.DiffPage.init.js:
On wikipage.diff hook fire, an item of this button select is selected according to the diffmode query parameter or the visualeditor-diffmode-historical user setting:

image.png (48×208 px, 1 KB)

This triggers a select event, and the onReviewModeButtonSelectSelect callback runs. There, new mw.Api().saveOption( 'visualeditor-diffmode-historical', mode ); initiates an API request.

Conclusion
So, generally, in 99.99% of cases, an option value is retrieved from mw.user.options.get( 'visualeditor-diffmode-historical' ), and then that exact value is saved back to the server. This request seems useless.

When I tried to make sense of this logic, I thought that maybe this behavior makes a difference in cases where multiple diffs are opened in different tabs, and the mode setting of the last opened tab should end up in the user settings. (The API request is postponed until the tab is first focused.) But perhaps I'm overthinking this :-)

Event Timeline

Great spot, @Jack_who_built_the_house – this in fact does seem like what it is happening is unnecessary. With this said, we will not have capacity to work on this in the next few months. If, by chance you feel compelled to submit patch, we would be happy to review it.

Change 748382 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Only use diffmode-historical user option when it has changed

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

Change 748382 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Only set diffmode-historical user option when it has changed

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

ppelberg claimed this task.