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

<?php
/**
* 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.

Files: 
CommentFileSizeAuthor
#7 correct-media-query-media-browser-alter-1824304-7-tests-only.patch3.16 KBDevin Carlson
FAILED: [[SimpleTest]]: [MySQL] 900 pass(es), 7 fail(s), and 0 exception(s).
[ View ]
#5 correct-media-query-media-browser-alter-1824304-5-tests-only.patch2.26 KBDevin Carlson
FAILED: [[SimpleTest]]: [MySQL] 851 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#5 correct-media-query-media-browser-alter-1824304-5.patch10.89 KBDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 907 pass(es).
[ View ]
#1 correct-media-query-media-browser-alter-1824304-1.patch785 bytesDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 5 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new785 bytes
PASSED: [[SimpleTest]]: [MySQL] 5 pass(es).
[ View ]

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.

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

Status:Reviewed & tested by the community» Fixed

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

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new10.89 KB
PASSED: [[SimpleTest]]: [MySQL] 907 pass(es).
[ View ]
new2.26 KB
FAILED: [[SimpleTest]]: [MySQL] 851 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

A year and a half later with tests!

The tests exposed a few other issues, so including the fixes in this patch.

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new3.16 KB
FAILED: [[SimpleTest]]: [MySQL] 900 pass(es), 7 fail(s), and 0 exception(s).
[ View ]

A better display of the test failures.

Status:Needs review» Needs work

It's perfectly save to leave in the media_hook_info() part. That just needlessly breaks implementations in custom code if removed.

Status:Needs work» Fixed

Fair enough. :)

Committed only the doxygen change from the original patch.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.