Page MenuHomePhabricator

Thanks comment says it handles FlowAddModules hook but it's actually a BeforePageDisplay hook handler.
Closed, ResolvedPublic

Description

Thanks.hooks.php says

/**
 * Handler for FlowAddModules.  Inserts javascript to enhance thank
 * links from static urls to in-page dialogs along with reloading
 * the previously thanked state.

but in gerrit 146169 the function was changed to onBeforePageDisplay() and it runs on the generic BeforePageDisplay hook. So fix the comment to describe how it currently works (easy).

If the code is intended to hook into FlowAddModules then add that as a @todo in the comment and keep this bug open. FlowAddModules might be slightly more performant (it wouldn't run on every page load) but presumably there's a reason @EBernhardson made the change :)

Event Timeline

Spage raised the priority of this task from to Normal.
Spage updated the task description. (Show Details)
Spage added a subscriber: Spage.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 16 2015, 9:32 PM
Spage updated the task description. (Show Details)Feb 16 2015, 9:40 PM
Spage set Security to None.
Spage added a subscriber: EBernhardson.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptJun 16 2015, 6:52 PM

Change 219985 had a related patch set uploaded (by Legoktm):
Fix doc comment for BeforePageDisplay hook

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

Change 219985 merged by jenkins-bot:
Fix doc comment for BeforePageDisplay hook

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

Legoktm closed this task as Resolved.Jun 22 2015, 9:20 PM
Legoktm claimed this task.