Doxygen for media_query_media_browser_alter is incorrect [#1824304]

The function media_query_media_browser_alter() in media.media.inc is incorrect. It states:

/**
 * Implements hook_query_media_browser_alter().
 */

but it actually implements hook_query_TAG_alter provided by system.module.

query_media_browser_alter is also listed as a media hook in media_hook_info. I believe this is incorrect and should be removed.

Comment	File	Size	Author
#7	correct-media-query-media-browser-alter-1824304-7-tests-only.patch	3.16 KB	devin carlson
#5	correct-media-query-media-browser-alter-1824304-5-tests-only.patch	2.26 KB	devin carlson
#5	correct-media-query-media-browser-alter-1824304-5.patch	10.89 KB	devin carlson
#1	correct-media-query-media-browser-alter-1824304-1.patch	785 bytes	devin carlson

Comments

Comment #1

devin carlson commented 26 October 2012 at 17:32

Status:

Active

» Needs review

Status	File	Size
new	correct-media-query-media-browser-alter-1824304-1.patch	785 bytes

A patch to correct the documentation and remove the hook from media_hook_info.

Since the function doesn't actually implement a media hook, I think it should be moved out of media.media.inc (probably into media.module), but I'll leave that for a separate issue.

Comment #2

ParisLiakos commented 26 October 2012 at 18:26

Status:

Needs review

» Reviewed & tested by the community

thanks.
not committing it yet, since i dont want to break a patch by accident that people in atlanta might review

Comment #3

ParisLiakos commented 27 October 2012 at 13:49

Status:

Reviewed & tested by the community

» Fixed

thanks commited
http://drupalcode.org/project/media.git/commit/a0d2c0d

Comment #4

ParisLiakos commented 29 October 2012 at 23:32

Status:

Fixed

» Needs work

i reverted this..removing the hook from hook_hook_info results to media_query_media_browser_alter() never being called..we need to move this function to the .module file

I would say we need tests for this..but i guess we need testbot first