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

Assigned To

Authored By

	matmarex
	Aug 3 2020, 6:39 PM

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:

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:

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

Details

	Subject	Repo	Branch	Lines +/-
	Ignore non-checkbox fields for page saving dialog	mediawiki/extensions/VisualEditor	REL1_35	+7 -0
	Ignore non-checkbox fields for page saving dialog	mediawiki/extensions/VisualEditor	master	+7 -0

Customize query in gerrit

Related Objects

Mentioned Here: rMW5901ac45dfd2: Localisation updates from https://translatewiki.net.
rMWd560bee54ee5: Merge "WatchAction: avoid unnecessary UPDATEs when expiry is unchanged"
T251348: Watchlist Expiry: Implement UI for VisualEditor [Large]

Event Timeline

matmarex created this task.Aug 3 2020, 6:39 PM

Restricted Application added projects: VisualEditor, Community-Tech. · View Herald TranscriptAug 3 2020, 6:39 PM

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

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

ifried updated the task description. (Show Details)Aug 3 2020, 9:04 PM

ppelberg subscribed.Aug 4 2020, 12:55 AM

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

ARamirez_WMF moved this task from Needs Discussion to Up Next (June 3-21) on the Community-Tech board.Aug 7 2020, 12:01 AM

ifried moved this task from Up Next (June 3-21) to Kanban-2020-21-Q1 on the Community-Tech board.Aug 7 2020, 12:09 AM

ifried edited projects, added Community-Tech (Kanban-2020-21-Q1); removed Community-Tech.

Samwilson claimed this task.Aug 7 2020, 2:50 AM

Samwilson moved this task from Ready 🎬 to In Development 💻 on the Community-Tech (Kanban-2020-21-Q1) board.

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

gerritbot added a project: Patch-For-Review.Aug 7 2020, 5:14 AM

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).

Samwilson moved this task from In Development 💻 to Review/Feedback 💬 on the Community-Tech (Kanban-2020-21-Q1) board.Aug 7 2020, 5:23 AM

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".

matmarex added a project: Editing-team (Kanban Board).Aug 10 2020, 5:44 PM

matmarex edited projects, added Editing-team; removed Editing-team (Kanban Board).

matmarex moved this task from Untriaged to Tracking on the Editing-team board.

matmarex edited projects, added Editing-team (Tracking); removed Editing-team.

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

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.4; 2020-08-11).Aug 10 2020, 7:00 PM

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).

Samwilson moved this task from Review/Feedback 💬 to QA 🐛 on the Community-Tech (Kanban-2020-21-Q1) board.Aug 14 2020, 2:11 AM

Dropdown no longer appears in Visual Editor.

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)

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptAug 14 2020, 9:13 PM

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