Last modified: 2011-03-13 18:06:39 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T25641, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 23641 - Pass &$watchthis as parameter to ArticleSaveComplete hook
Pass &$watchthis as parameter to ArticleSaveComplete hook
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
Page editing (Other open bugs)
unspecified
All All
: Lowest enhancement (vote)
: ---
Assigned To: Happy-melon
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-24 06:02 UTC by tisane2718
Modified: 2011-03-13 18:06 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Patch to Article.php fixing this bug (4.04 KB, patch)
2010-05-25 09:00 UTC, tisane2718
Details
Patch to Article.php fixing this bug and bug 23655 (5.65 KB, application/octet-stream)
2010-05-25 11:17 UTC, tisane2718
Details
Fix items from message #5. (5.31 KB, patch)
2010-05-25 11:42 UTC, tisane2718
Details

Description tisane2718 2010-05-24 06:02:06 UTC
This will enable the hook function to control whether a page will be watched or unwatched. This is needed so that, for instance, the PureWikiDeletion extension's "Add pages I blank to my watchlist" preferences-based functionality can work.
Comment 1 Happy-melon 2010-05-24 15:20:39 UTC
If you want to write a patch for this, I'll review and commit it.  My workload is escalating up to the last week in June, however, so I may become less available as time goes by.
Comment 2 tisane2718 2010-05-25 00:05:09 UTC
OK, I'll start work on that. This will probably be done as part of the same patch that fixes bug 23655.
Comment 3 tisane2718 2010-05-25 09:00:05 UTC
Created attachment 7402 [details]
Patch to Article.php fixing this bug

This changes $watchthis to &$watchthis so that hooks (ArticleSave, ArticleInsertComplete, and ArticleSaveComplete) can change its value. It also introduces $alterWatchlist, which if null causes $watchthis to be ignored. The alterations to the watchlist are now done by doEdit rather than by insertNewArticle or updateArticle, which I guess moves us a step closer to deprecating those two functions.
Comment 4 tisane2718 2010-05-25 09:22:24 UTC
Actually, I think I just figured out how to fix bug 23655, so I'll be submitting a substitute patch to fix both.
Comment 5 Happy-melon 2010-05-25 11:14:34 UTC
-		$this->doEdit( $text, $summary, $flags );
+		$this->doEdit( $text, $summary, $flags, $false, null, $watchthis, true );

"$false"??  Please test your code before submitting.  

+	 * @param $watchthis Unwatch the page if null, otherwise watch it; ignored if $alterWatchlist is null
+	 * @param $alterWatchlist If null, watchlist will be left alone; otherwise page will be watched or unwatched,
+	 *             depending upon value of $watchthis

It would be nicer to implement this as one parameter $watch, where the page is watched if true, unwatched if false, and left unchanged if null.  Right now there are no hooks in SVN making use of the existing $watchthis parameter, so there's no harm in slightly tweaking its implementation.
Comment 6 tisane2718 2010-05-25 11:17:31 UTC
Created attachment 7403 [details]
Patch to Article.php fixing this bug and bug 23655

This patch moves most of the functionality of insertNewArticle and updateArticle into doEdit, which should make it pretty easy to modify the rest of the core so as to deprecate those former two functions (although that'll be another patch). &$redirect is introduced, which allows the hook function to choose whether or not the user will be redirected back to the page after the edit has been successfully committed.
Comment 7 tisane2718 2010-05-25 11:21:45 UTC
I can fix this patch to implement your suggestions from comment 5, or wait till you've reviewed the second patch and then create a third patch fixing whatever additional problems you may point out; whichever you prefer.
Comment 8 tisane2718 2010-05-25 11:42:05 UTC
Created attachment 7404 [details]
Fix items from message #5.
Comment 9 tisane2718 2010-05-25 13:31:09 UTC
I wonder if I am taking the right approach in folding most of the code from the other functions into doEdit, or whether it would be a better idea to fold it out into the functions that call insertNewArticle and updateArticle?
Comment 10 Happy-melon 2010-05-25 13:46:52 UTC
If your changes reduce code duplication, which they seem to do; then you probably have the right approach.  It's pretty rare that increasing duplication is the right thing to do.
Comment 11 Happy-melon 2010-05-25 16:41:48 UTC
Patch applied in r66878.  Does this also fix bug 23655?
Comment 12 tisane2718 2010-05-25 16:53:15 UTC
Yes, it does. Thanks for your help.
Comment 13 Tim Starling 2010-08-09 08:38:06 UTC
Reverted r66878.

Add another hook, don't use ArticleSaveComplete for this.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links