VE could also launch the popup-based reauth workflow. That shouldn't be too difficult to integrate.

In T432181#12124574, @Priyanshikaa1111 wrote:

I would like to claim this task and start working on it.

Ok. I think the simplest approach would be adding the VE hook to CommonSettings.php with a check for $title->isRawHtmlMessage(). So it probably makes sense to ship that first, since we want to improve upon these issues this quarter (July 1st - September 30th). And then maybe better-integrating the re-auth popup / fallback into VE after that, as @matmarex suggests.

If you could let us know of your progress on this next week, that would be great, so that we can provide code review and get this deployed in a timely fashion. Thanks.

In T432181#12124339, @matmarex wrote:

VE could also launch the popup-based reauth workflow. That shouldn't be too difficult to integrate.

Not using VE for these pages is probably an improvement in its own right. $wgRawHtmlMessages pages are HTML and VE would try to render and edit them as wikitext, which is very confusing. Gadgets-definition is kinda-sorta wikitext, but not to the extent where using VE would actually be useful, IMO.

(That said, nice reauth handling in VE would still be nice. There have been discussions about requiring reauth for templates with very high inclusion counts, and third-party wikis might require reauth for editing protected wikitext pages or whatever.)

VE wouldn't make sense (and it's disabled in the 'MediaWiki:' namespace), but VE's wikitext editor ("2017 wikitext editor") can open and edit these pages. Sorry, I wasn't clear what I meant. It can be enabled in preferences, or you can try it like this: http://mediawiki.org/wiki/MediaWiki:Gadgets-definition?veaction=editsource

Currently it fails with this message:

Yeah, the proper fix is to integrate re-auth workflow in 2017NWE, since some interface admins do prefer using it for wikitext editing (not me personally, but nonetheless).

Thanks for the clarification! I'll implement the check using $title->isRawHtmlMessage() in the VE hook so VisualEditor's visual mode is blocked on these pages, while keeping the 2017 wikitext editor available. Will share a patch for review soon.

Should this check go in CommonSettings.php (mediawiki-config repo) or inside the VisualEditor extension itself? I want to make sure I'm patching the right repo before proceeding.

Hi! I've set up both the VisualEditor extension and mediawiki-config repos locally, and confirmed the existing VisualEditorBeforeEditor hook in includes/Hooks.php. Before writing the patch, I want to confirm: should the isRawHtmlMessage() check be added as a new hook registration in CommonSettings.php (mediawiki-config), or implemented as a hook handler inside the VisualEditor extension itself? Want to target the right repo for the Gerrit patch.

In general, if you want to add a new Wikimedia-specific hook handler, the right place for that is the WikimediaCustomizations extension, not mediawiki-config (even though it still has a bunch of legacy handlers).

In this case though, I think all the reauth logic has been moved to core and is not Wikimedia-specific, so the VisualEditor extension is the better place.

I might be misunderstanding something, but I don't think we need to create a new hook. I think we should use VE's existing VisualEditorBeforeEditor hook (seems like a fine point in the execution to handle this) and place that in CommonSettings.php (where several other legacy hook blocks reside) but pushing it onto $wgHooks or possibly just adding it to WikimediaCustomizations, if we think that's a better location.

The isRawHtmlMessage() check can just be done in VisualEditor, I don't think it requires a hook.

In T432181#12139095, @Tgr wrote:

The isRawHtmlMessage() check can just be done in VisualEditor, I don't think it requires a hook.

Ok, fair enough.

I've submitted a patch implementing this:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/1317621
Added a check for isRawHtmlMessage() directly in includes/Hooks.php's onBeforePageDisplay(), per Tgr's suggestion. Would appreciate a review when you have a chance!

Hi @sbassett @Tgr, I've submitted a patch for this: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/1317621
Would appreciate a review!

Jumping in here now that I've been pinged on a patch, I agree with @matmarex and disagree with this task.

"VE" in this case means the 2017 wikitext editor, of course, which doesn't ever render HTML except in "Visual" diff preview mode when saving.

I think this task is mistaken. The source editor should work with all editable wikitext and wikitext-ish surfaces on-wiki, and should not be blocked in this case.

If you find a wiki where the MediaWiki: namespace is configured to be VE-editable (visual mode), that is. production configuration bug.

@Priyanshikaa1111: Hi, in general, please closely follow https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines, then there is no need to ever manually announce your patch in a task. Thanks.

Change #1317621 had a related patch set uploaded (by Aklapper; author: @Priyanshikaa1111):

[mediawiki/extensions/VisualEditor@master] Hooks: Block VisualEditor on raw HTML message pages

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

In T432181#12157722, @Jdforrester-WMF wrote:

Jumping in here now that I've been pinged on a patch, I agree with @matmarex and disagree with this task.

I think that's ultimately where we want to be, yes, with reauth properly implemented for various sensitive editing via VE. But as an interim solution (that is hopefully fairly temporary), we were thinking this approach, as implemented via @Priyanshikaa1111's patch, would be a reasonable way to stave confusion and minimally address a currently-poor UX.