Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comments
Comment #1
ksenzeeI'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.
Comment #2
Dave ReidHrm, 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?
Comment #3
ksenzeeWell. 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.
Comment #4
ksenzeeHere's a patch for 1.x if that's useful at this point.
Comment #6
jarrodirwin CreditAttribution: jarrodirwin commented#3: 1307596-3.browser-validation-behavior-2.x.patch queued for re-testing.
Comment #7
idflood CreditAttribution: idflood commentedHere is a reroll of patch in #3, one function was in a different file. I've tested it a little and it worked nicely.
Comment #8
idflood CreditAttribution: idflood commentedComment #9
Dave ReidComment #10
dddave CreditAttribution: dddave commented#7: 1307596-6-browser-validation-behavior-2.x.patch queued for re-testing.
Comment #12
katbailey CreditAttribution: katbailey commentedHere's a reroll of the 1.x version
Comment #13
Dave ReidComment #14
raytiley CreditAttribution: raytiley commentedHere 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.
Comment #15
raytiley CreditAttribution: raytiley commentedComment #16
raytiley CreditAttribution: raytiley commentedFixing for coding standards in above patch.
Comment #17
raytiley CreditAttribution: raytiley commentedThis patch also applies cleanly to 1.1 and fixes the validation tab selection problem.
Comment #18
Dave ReidI'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.
Comment #19
Dave ReidI 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
Comment #20
gbernier CreditAttribution: gbernier commentedHey 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
Comment #21
mrfelton CreditAttribution: mrfelton commented#20 also related to #1802026: Ajax callbacks in the media overlay cause the active tab to switch tab unexpectedly
Comment #22
mrfelton CreditAttribution: mrfelton commentedThis 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.
Comment #23
mrfelton CreditAttribution: mrfelton commentedActually, 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 calledembed_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:
to something like:
Comment #24
ParisLiakos CreditAttribution: ParisLiakos commentedThanks, 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
Comment #25
JBodkin CreditAttribution: JBodkin commentedSame here, I had clientside validation enabled, I added "media/*" to the exclude path in the module configuration and it seems to be working correctly.
Comment #26
Devin Carlson CreditAttribution: Devin Carlson commentedThis should be fixed with #2108647: Array.indexOf doesn't exist!.