Page MenuHomePhabricator

Watchlist Expiry: Fix interface bug in VisualEditor [[MEDIUM]]
Closed, ResolvedPublic

Description

Background: Temporary watchlist options interface appears broken in VisualEditor, due to our Watchlist Expiry changes.

Acceptance Criteria:

  • Restore previous VisualEditor behavior, in which only the original watchlist checkbox is displayed

Visual Examples:

The interface when saving changes in VisualEditor looks like this:

image.png (2×3 px, 450 KB)

There is an unexpected checkbox next to "Watchlist time period:", and the dropdown that should be there instead is missing. (Checking or unchecking that checkbox does nothing.) The "Watch this page" checkbox works as normal.

Source editor interface, for comparison:

image.png (2×3 px, 347 KB)

Expected behavior: either the same interface appears as in source mode (related task: T251348), or at least there aren't any non-functional checkboxes.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
ifried added a subscriber: ifried.

Thanks for catching this, @matmarex! Yup, we'll need to fix this. I'll discuss it with the team.

ifried renamed this task from Temporary watchlist options interface appears broken in VisualEditor to Watchlist Expiry: Fix interface bug in VisualEditor.Aug 6 2020, 3:24 PM

It probably wouldn't be too much work to make it work properly, actually. VisualEditor shares most of the code used for generating the "edit checkboxes" (which you've extended by adding the dropdown), that's why the field showed up in VE in the first place.

You'll probably just need to make similar changes in ArticleTargetLoader#createCheckboxFields as you've made in EditPage.php in MediaWiki to make it render as a dropdown, and in ArticleTarget#getSaveFields to not treat it like a checkbox.

ARamirez_WMF renamed this task from Watchlist Expiry: Fix interface bug in VisualEditor to Watchlist Expiry: Fix interface bug in VisualEditor [[MEDIUM]].Aug 6 2020, 11:59 PM

Change 618880 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/VisualEditor@master] Ignore non-checkbox fields for page saving dialog

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

Thanks @matmarex, that's super helpful. I've made a little patch to ignore the dropdown for now, because that's what this task calls for and because there's a bunch of other handling that needs to be done at the same time to properly handle watchlist expiry (e.g. enabling/disabling the dropdown, and changing its value when the expiry is changed via the watch-this content action link).

I think as part of extending "checkboxes" to include other fields, the various public methods and variables should be renamed from "checkboxes" to "(extra/additional/other)Fields".

Change 618880 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Ignore non-checkbox fields for page saving dialog

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

I think as part of extending "checkboxes" to include other fields, the various public methods and variables should be renamed from "checkboxes" to "(extra/additional/other)Fields".

I totally agree. I tried doing some of that, but there's a bunch of places that assume the 'checkboxes' name (such as a hook, and methods that are overridden in child classes).

dom_walden added a subscriber: dom_walden.

Dropdown no longer appears in Visual Editor.

ve.png (355×515 px, 31 KB)

Tested with Watchlist Expiry enabled and disabled. All looked fine. No JavaScript errors. Watching and unwatching functions fine.

I tested mainly on Vector, but briefly looked at it on Monobook and Timeless. Also looks fine on Mobile.

Test environment https://en.wikipedia.beta.wmflabs.org MediaWiki 1.36.0-alpha (d560bee) 2020-08-14T09:17:05. Local vagrant MediaWiki 1.36.0-alpha (5901ac4).

I have also tested this on testwiki, and it looks good (screenshot example below). I'm marking this as Done.

Screen Shot 2020-08-14 at 5.12.14 PM.png (391×1 px, 72 KB)

Change 626528 had a related patch set uploaded (by Dmaza; owner: Samwilson):
[mediawiki/extensions/VisualEditor@REL1_35] Ignore non-checkbox fields for page saving dialog

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

Change 626528 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_35] Ignore non-checkbox fields for page saving dialog

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