Wikimedia Bugzilla is closed!

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.