Just dumping notes here 'cause I'm lazy and don't want to lose them.

• Fix creation edit summary
• Categorise all delivery lists (addTrackingCategory?)
• Kill add pages header
• Link back to list
• Fix instructions on special page and don't show technical details for single wiki
• Add copyright disclaimer (see Special:MassMessage)
• Mention protected status on the special page
• Block interwiki titles (Title::isExternal) and special pages
• Escape wikitext in invalid targets error (wfEscapeWikitext) ---

Additional thoughts I had:

• edit notices? blergh
• icons instead of words such as "(remove)" might be cleaner; dunno
• still think the "Add pages" header should be killed; would like to discuss further
• it's a bit weird to see "Pages in this list" as an h2 without a section-edit link
  • if the "Add pages" header is kept, it should maybe have a section-edit type link that reads "[bulk edit]" or similar
• make "(remove)" less dangerous
  • probably depends on https://gerrit.wikimedia.org/r/92315

I'm largely done fixing the things in MZMcBride's comment. Some comments about the ones that I have not implemented as described:

• Add pages form / header: Form moved to before the page list per IRC discussion (prevents people having to scroll down to get to it on long lists)

• Icons instead of words such as "(remove)": I've generally avoided adding custom design elements in order to ensure compatibility with skins other than Vector; if others also feel that an icon here would be better, this can probably be done.

• Section edit links: Personally I don't think they make sense in this context...

https://github.com/wikimedia/mediawiki-extensions-MassMessage/compare/contenthandler

So looking at MassMessagehooks.php

$out->addModuleStyles( 'ext.MassMessage.content' );
$out->addModules( 'ext.MassMessage.content.js' );

A module ending in .js just feels icky. That's alright though, as the 2 should be merged, and then we can just have:

$out->addModules( 'ext.MassMessage.content' );

Api Modules:
getPossibleErrors is no more.

Other:
There are usages of "else if" in php code. Don't do that. Use "elseif"

(In reply to Sam Reed (reedy) from comment #3)

https://github.com/wikimedia/mediawiki-extensions-MassMessage/compare/
contenthandler

So looking at MassMessagehooks.php

$out->addModuleStyles( 'ext.MassMessage.content' );
$out->addModules( 'ext.MassMessage.content.js' );

A module ending in .js just feels icky. That's alright though, as the 2
should be merged, and then we can just have:

$out->addModules( 'ext.MassMessage.content' );

We have to do that so the CSS styles are still served to users with JS disabled. Modules loaded via addModules require JS. Also there would be a FOUC iirc.

SpecialCreateMassMessageList and SpecialEditMassMessageList doesn't return something on all code paths in their onSubmit()

MassMessageHooks.php - Line 192
$revId = ( $next ) ? $next : $revId;

$revId on the right hand side is undefined

ApiEditMassMessageList::getEditSummary() - $title and $count used at the bottom may have not been defined (I note there's quite a few code paths through that block...)

MassMessageListContentHandler::unserializeContent() should have @throws MWContentSerializationException

(In reply to Sam Reed (reedy) from comment #3)

Api Modules:
getPossibleErrors is no more.

Id74b59113cf0211e83c25364a94c597cb579b8f0

Other:
There are usages of "else if" in php code. Don't do that. Use "elseif"

I0f637f466b7c41053c9ba0a8d9399292c843104b

(In reply to Sam Reed (reedy) from comment #5)

MassMessageListContentHandler::unserializeContent() should have @throws
MWContentSerializationException

I62c73abd5f9e3fd4c2e9358e484101b962cdc6c0

(In reply to Sam Reed (reedy) from comment #5)

SpecialCreateMassMessageList and SpecialEditMassMessageList doesn't return
something on all code paths in their onSubmit()

I3a06dd743ff8b18aaba73649cbd53d4db02018ce

ApiEditMassMessageList::getEditSummary() - $title and $count used at the
bottom may have not been defined (I note there's quite a few code paths
through that block...)

AFAIS everything is defined properly, but explicitly done in I3393a7a5531c272cf879bad0da11426714a0e633.

(In reply to Sam Reed (reedy) from comment #5)

MassMessageHooks.php - Line 192
$revId = ( $next ) ? $next : $revId;

$revId on the right hand side is undefined

I think this should be $oldid, but I'm not totally sure...wctaiwan?

(In reply to Kunal Mehta (Legoktm) from comment #8)

(In reply to Sam Reed (reedy) from comment #5)

MassMessageHooks.php - Line 192
$revId = ( $next ) ? $next : $revId;

$revId on the right hand side is undefined

I think this should be $oldid, but I'm not totally sure...wctaiwan?

$oldid is correct; sorry. Changed in I75cb898dcd20db8052688152d55a8f3d63e9e241

I7e01e63717b99fe6937fa8598fdaffe8fdfe4527 closes a couple more loopholes for invalid titles, with thanks to MZMcBride for raising them.

Marking as fixed as mm-ch is now in master.