If you have a form validation error in one of the media browser tabs, the media browser reloads. At that point you should see the tab that threw the error, not whichever tab happens to be first.

Files: 
CommentFileSizeAuthor
#23 media-error_tab_selection_2.x-1307596-23-followup.patch1.84 KBmrfelton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch media-error_tab_selection_2.x-1307596-23-followup.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#16 media-error_tab_selection_2.x-1307596-reroll.patch1.04 KBraytiley
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to create dependency directory.
[ View ]
#14 media-error_tab_selection_2.x-1307596.patch1.07 KBraytiley
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch media-error_tab_selection_2.x-1307596.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#12 1307596-12.browser-validation-behavior-1.x.patch7.02 KBkatbailey
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-12.browser-validation-behavior-1.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#7 1307596-6-browser-validation-behavior-2.x.patch7.05 KBidflood
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-6-browser-validation-behavior-2.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#4 1307596-4.browser-validation-behavior-1.x.patch6.59 KBksenzee
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-4.browser-validation-behavior-1.x.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#3 1307596-3.browser-validation-behavior-2.x.patch6.77 KBksenzee
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-3.browser-validation-behavior-2.x.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 1307596-1.browser-validation-behavior-2.x.patch5.54 KBksenzee
PASSED: [[SimpleTest]]: [MySQL] 37 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new5.54 KB
PASSED: [[SimpleTest]]: [MySQL] 37 pass(es).
[ View ]

I'm attaching a patch that should take care of this. The JavaScript is a little more indirect than you would expect. The server-side code knows that it's thrown an error, and that the tab it provides needs to be active on the next page load. It can pass that information to the client via Drupal.settings. But the jQuery UI Tabs library isn't really set up to select a tab via a setting; it wants the URL hash to be the title of the tab that should be preselected as active. We can't do that. So we have to use the ugly backdoor API, which is specifying the integer index of the tab to select (i.e. "select the second tab in the list"). Hence the somewhat ugly JS.

Hrm, wondering what happens if you submit the 'Web' tab with an error, then then instead of fixing it, go to another tab 'Upload' and submit it without selecting a file? What tab does it go to then?

StatusFileSize
new6.77 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-3.browser-validation-behavior-2.x.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Well. Two problems there:

1. I forgot to update the 'Upload' plugin in my 2.x version of the patch, so it wasn't setting itself to active after an error. (Yes, I'm still writing 1.x patches first. :/ )

2. I didn't realize form_get_errors() was as stupid (i.e. global) as it is. Apparently we need a helper function that iterates over the form to check every field for errors.

StatusFileSize
new6.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-4.browser-validation-behavior-1.x.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Here's a patch for 1.x if that's useful at this point.

Status:Needs review» Needs work

The last submitted patch, 1307596-4.browser-validation-behavior-1.x.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work
StatusFileSize
new7.05 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-6-browser-validation-behavior-2.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here is a reroll of patch in #3, one function was in a different file. I've tested it a little and it worked nicely.

Status:Needs work» Needs review

Issue tags:+Needs backport to 1.x

Status:Needs review» Needs work
Issue tags:+Needs backport to 1.x

The last submitted patch, 1307596-6-browser-validation-behavior-2.x.patch, failed testing.

StatusFileSize
new7.02 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1307596-12.browser-validation-behavior-1.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here's a reroll of the 1.x version

Issue tags:+sprint, +Media Initiative

StatusFileSize
new1.07 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch media-error_tab_selection_2.x-1307596.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here is a patch for the 2.x version that fixes the tab selection on validation errors only changing the javascript, not touching the form api or anything.

Status:Needs work» Needs review

StatusFileSize
new1.04 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to create dependency directory.
[ View ]

Fixing for coding standards in above patch.

This patch also applies cleanly to 1.1 and fixes the validation tab selection problem.

I'm happy with just simple JavaScript for this for now. I'll keep this open as I'd like to be able to set the default tab programatically, or a better solution either in media.browser.inc or a potential MediaBrowserFormPlugin() that could automatically do the backend-processing we're seeing in this issue, but on a more generic basis.

Ray and I also discussed that there's an edge-case if you cause errors on multiple tabs (one after the other). And that it would also possibly be nice if the Tabs themselves were marked with an error star to indicate an error was visible in that tab. We can leave those as followups.

Status:Needs review» Needs work

I committed #16 to both 7.x-2.x and 7.x-1.x. Thanks everyone for the great work so far!
http://drupalcode.org/project/media.git/commit/4730421
http://drupalcode.org/project/media.git/commit/536c100

Hey Guys,

Noticing with modules which have their own Media Browser plugins and search forms, such as YouTube, when you do the search or the pagination it doesn't take you page to the proper tab either. This seems to be related to the same issue, we are using 7.x-2.0-unstable6+9-dev Media currently

This no longer works. Error messages are rendered at the top of the media browser page as opposed to within the individual tabs, so trying to work out which tab the errors relate to based om their position in the DOM doesn't work.

Status:Needs work» Needs review
StatusFileSize
new1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch media-error_tab_selection_2.x-1307596-23-followup.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Actually, there were 2 parts to my problem.

1) I had the clientside_validation module enabled, it was causing an empty div with the class .error to be injected into every tab of the media browser. This screwed up the js from the previous patches, because it would interpret this an error en every tab, and thus switch to one of them when the form was submitted, or an ajax widget was used (like the file upload button on the upload tab).

2) There is a bug in the media_internet_sources code that sets a form error on the wrong element. It sets the error on an element called url but there is no such element. The form item is actually called embed_code. This means that when there was an error using the internet sources tab, the offending field woud not be given the .error class, causing the js here to fail to work out which tab the error was in.

Attached patch resolves the issue in 2), correct the form_set_error() calls so that they set the error on the right element.

To fix 1), I ended up just excluding media/browser from clientside_validation. But I thought this may also be fixable by changing:

var errorTabID = $('#media-browser-tabset')
    .find('.error')
    .parents('.media-browser-tab')
    .attr('id');

to something like:

var errorTabID = $('#media-browser-tabset')
    .find('.error:visible')
    .parents('.media-browser-tab')
    .attr('id');

Status:Needs review» Needs work

Thanks, i tested it manually and works.
Committed the patch:)
http://drupalcode.org/project/media.git/commit/5977414
Back to needs work according to #18

Also committed to 1.x since there was the same issue there as well
http://drupalcode.org/project/media.git/commit/f8bfe9a

Same here, I had clientside validation enabled, I added "media/*" to the exclude path in the module configuration and it seems to be working correctly.

Status:Needs work» Fixed

This should be fixed with #2108647: Array.indexOf doesn't exist!.

Status:Fixed» Closed (fixed)

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