User "Catrope" changed the status of MediaWiki.r94268. Old Status: new New Status: fixme
User "Catrope" also posted a comment on MediaWiki.r94268. Full URL: https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/94268#c20661 Commit summary: ajaxCategories fixes based on review in r93351 CR: * Using typeof check in clean() * Use mw.Title to get page title from fullpagename instead of split(':') * replaceNowikis() and restoreNowikis() - Improve documentation - Moved dash in the UNIQUEKEY to between the id and the incrementing integer, and made it start with an empty string (so that all following concatenations are toString'ed). * makeCaseInsensitive(): Moved the wgCaseSensitiveNamespaces-check out and wrapped it around the caller instead. Also cached the outcome of "Is category namespace sensitive ?". * createButton(): text-argument is indeed text, not html. Applying html-escaping. * resolveRedirects(): - Replace access to private property _name of mw.Title with function getMainText(). * handleCategoryAdd() and handleEditLink(): - Restructure title-handling (no local replace() calls and clean(), let mw.Title handle it) - Renaming arguments and documenting them better - Renaming local variables and removing redundant parts - Preserving sortkey as sortkey as long as possible without the pipe - Calling the combination of sortkey and leading pipe 'suffix' instead of, also, sortkey. * createCatLink(): - Remove the sanitizing here, the string passed is already clean as it comes from mw.Title now - Using .text() instead of .append( which is .html-like), category names can contain special characters. * containsCat(): - Using $.each instead of [].filter. Stopping after first match. * buildRegex(): Allow whitespace before namespace colon, and allow whitespace after category name (but before ]] and |..]]) Additional changes not for any function in particular: * Literally return null in $.map callbacks. * Using the existence-system of mw.Title instead of passing around booleans everywhere ** Removed 'exists' argument from the resolveRedirects() and handleCategoryAdd() functions, instead checking .exists() of the mw.Title object. * Passing and using mw.Title objects where possible instead of converting back and forth between strings and objects etc. * Using "TitleObj.getUrl()" instead of "catUrl( titleString )". Removed now unused catUrl() function. * To improve readability, renamed local uses of 'var that = this' to 'var ajaxcat = this'. * Syntax error fixes (.parent -> .parent()) * Merging var statements * Renamed generic members of 'stash' from 'stash.summaries' to 'stash.dialogDescriptions' and 'stash.shortSum' to 'stash.editSummaries'. dialogDescription is always HTML (input should be escaped before hand) Comment: <pre> + // Redirect existence as well (non-existant pages can't be redirects) + mw.Title.exist.set( catTitle.toString(), true ); </pre> That's wrong. You're setting the existence of the redirect '''target''', not the redirect '''itself'''. Redirects themselves can't be nonexistent, but redirect targets sure can. The logic in the entire function is backwards anyway: <code>exists</code> will reflect whether the redirect target exists, in case of redirect resolution. <pre> - // Readd static. + // Read static. </pre> I'm pretty sure that wasn't a typo and was supposed to say 'readd' (as in 'add again') or maybe 're-add', but not 'read'. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
