Duplicate cancel button on multiple file edit page. [#1974554]

We have two cancel buttons on the file edit page. See the attaches screenshot. The Cancel button is added by media browser plus module and the Cancel link is added by the media module. We have to remove one of them.

Comment	File	Size	Author
#6	media_browser_plus-drupal_alter_added-1974554-6.patch	397 bytes	boychev
#6
#3	Edit multiple files kk dam.local_.png	153.49 KB	boychev
#1	media_browser_plus-duplicate_cancel_button-1973904-0.patch	526 bytes	boychev
#1
	Edit multiple files kk dam.local_.png	30.75 KB	boychev

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

boychev CreditAttribution: boychev commented 19 April 2013 at 08:36

Status:

Active

» Needs review

File	Size
media_browser_plus-duplicate_cancel_button-1973904-0.patch	526 bytes

I've made a patch for this issue which checks if there is two cancel buttons and if yes - removes one of them. In my case it is better to remove the cancel link, provided by the media module because it does not work properly anyway.

Comment #2

das-peter CreditAttribution: das-peter commented 19 April 2013 at 13:21

Status:

Needs review

» Needs work

Are you sure the patch works?
I'm asking because:

I can't find a cancel link / button with an uppercase C (Cancel) as array key, whether in MBP nor Media (version from 09.02.2013)
MBP defines $form['actions']['cancel'] -> "actions", the condition in the patch compares only "buttons".
So none of the buttons seems to come from MBP -> not an MBP bug?

Btw. the patch contains trailing slashes. I recommend to configure (checkout http://drupal.org/node/147789) your IDE to automatically remove trailing slashes to meet the Drupal Coding Standards. A great help is the Code Sniffer in the Coder Module (7.x-2.x branch).

Comment #3

boychev CreditAttribution: boychev commented 19 April 2013 at 13:59

File	Size
Edit multiple files kk dam.local_.png	153.49 KB

Thanks for recommendations. I attached a screenshot with the all buttons that media_file_page_edit_multiple() function returns in the form. ...['Cancel'] button is the submit button, ...['cancel'] button is the link. I also think this is not a MBP plus bug. Here is the declaration of the cancel button in MBP:

$form['actions']['cancel'] = array(
    '#type' => 'submit',
    '#value' => t('Cancel'),
    '#weight' => 20,
    '#submit' => array('media_browser_plus_edit_cancel'),
  );

I'm wondering from where the ['Cancel'] item comes.

Comment #4

das-peter CreditAttribution: das-peter commented 19 April 2013 at 14:06

A wild guess: "Crop" is written with an uppercase "C" - which module adds this button? Does it add other buttons too? E.g. Save, Delete ...

Comment #5

boychev CreditAttribution: boychev commented 19 April 2013 at 14:51

Disabling the module that provides crop functionality does not resolve our problem. I'll make a detailed research from where this should come. Thanks for the effort.

Comment #6

boychev CreditAttribution: boychev commented 21 April 2013 at 12:55

File	Size
media_browser_plus-drupal_alter_added-1974554-6.patch	397 bytes

I think it will be great to have drupal_alter() function here if we want to make some modifications to the edit multiple form.

Comment #7

das-peter CreditAttribution: das-peter commented 22 April 2013 at 05:55

Well, basically media_browser_plus_edit_multiple_form is already an alteration of media_file_page_edit_multiple.
Overwrite the menu item with hook_menu_alter(&$items), register your own page callback and there you go - make what ever you'd like in the wrapper function.
But I'm not sure we should encourage developers to mess up this stuff even more by providing an easier way to do so :P
Further, this function was removed in the MBP 3.x branch - so it won't be a long-term solution either.

Comment #8

boychev CreditAttribution: boychev commented 22 April 2013 at 10:39

hook_menu_alter also can do the work, but in this case we should rewrite the existing code and there will be duplicated code. In my opinion it is always good to add alter hooks when you build a form.

Comment #9

das-peter CreditAttribution: das-peter commented 22 April 2013 at 11:39

Actually I want to get rid of MBP 2.x as soon as possible. The whole branch was a pile of code duplication and I stuck with it just because I hadn't time to rewrite it.
Now as I invested quite some time into the 3.x branch I'll prefer to focus on that and not on cleaning up the fubar 2.x version.
I've you really want to stay with the 2.x branch I would happily add you as maintainer - but unfortunately I don't have the permissions to administer maintainers for this project.