Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#18764 closed defect (bug) (fixed)

no CSS ID or class or hook for themes/plugins to modify "max file size" paragraph in media_upload_form()

Reported by: jaredh123's profile jaredh123 Owned by: azaozz's profile azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Media Keywords: close
Focuses: Cc:

Description

in wp-admin/includes/media.php line 1534 (as of svn# 18763), there is a paragraph that says "Maximum upload file size is X... after file has been uploaded, you can add titles and descriptions."

For themes are plugins that are using media_upload_form(), this paragraph might make no sense for the context they are using it. There is no way to filter this paragraph, nor even a usable CSS class or ID to target it with CSS. The only current way to target this paragraph and remove it or modify it is by using jQuery to find the text and hide it.

Possibly a filter, or at the very least a CSS selector would help theme and plugin authors. Thanks!

Attachments (3)

ticket_18764.diff (629 bytes) - added by jaredh123 12 years ago.
proposed patch with simple addition of CSS id
18764-with-classes.diff (1.8 KB) - added by jaredh123 12 years ago.
18764.diff (1.9 KB) - added by scottbasgaard 12 years ago.
Hooked into 'post-upload-ui' as advised by @nacin.

Download all attachments as: .zip

Change History (21)

@jaredh123
12 years ago

proposed patch with simple addition of CSS id

#1 @jaredh123
12 years ago

  • Keywords has-patch added

#2 @jaredh123
12 years ago

  • Version 3.3 deleted

#3 @jaredh123
12 years ago

  • Version set to 3.3

#4 follow-up: @nacin
12 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 3.3

We keep adding messages here, including a few in 3.3. I don't like it. Each should instead be hooked into one of the actions, like post-upload-ui.

#5 @scottbasgaard
12 years ago

  • Keywords has-patch added; needs-patch removed

#6 @jaredh123
12 years ago

This looks good. Could we make one more change and just put a class on each of the two paragraphs in the media_upload_max_file_size_message() func?

It would be a common scenario where a plugin or theme author would like to keep the max-file size message, but the "you may add titles and descriptions" would not make any sense. With this patch, a developer would have to still resort to hacky javascript or non-backward compatible advanced CSS3 just to hide that one paragraph, or be forced remove function from the post-upload-ui action entirely.

Adding a couple of simple classes would solve this. See my modification ot scottbasgaard's patch attached.

#7 @scottbasgaard
12 years ago

I agree with you jared.

I think that that would be plugin/theme territory. With the patch I submitted above you could do the following. I.e In a functions.php file:

function custom_media_upload_max_file_size_message() {
	// Do your custom message here.
}
remove_action('post-upload-ui', 'media_upload_max_file_size_message');
add_action('post-upload-ui', 'custom_media_upload_max_file_size_message');

#8 @jaredh123
12 years ago

So can we also add the classes to the two paragraphs? I'm a bit confused. You say you agree with me, but what I'm asking is that we also add class attributes to the two paragraphs.

I agree that a theme/plugin author can remove the action and write their own action for a custom message, but what if they want the max file size message but not the "after you upload you can add titles and descriptions" message. Then they would be forced to rewrite the first part just to exclude the second.

So, what I think I'm getting at is, this function: media_upload_max_file_size_message() is doing two things, it's outputting the max file size message, but it's also outputting a message about adding titles and descriptions after upload. We either need to a) separate those two messages out, or at the very least b) put classes on each of the paragraphs so a simple CSS rule could hide the second option. Do you see what I'm saying? The way it's written now, the function should be called media_upload_max_file_size_and_post_upload_instructions_messages().

Let's either split those apart, or just add some classes to the two paragraphs outputted by the function as is. There are lots of occasions where the max-file size message makes sense, but not the second paragraph. The only difference between my .diff and yours is I added two class attributes.

@scottbasgaard
12 years ago

Hooked into 'post-upload-ui' as advised by @nacin.

#9 @scottbasgaard
12 years ago

  • Cc mail@… added

Revised my patch a bit. What do you think jared?

Let's make this text filterable rather than adding an id/class.

You could then do something like this.

function custom_text($text) {
	return 'Custom text here.';
}
add_filter('media_upload_title_and_description_message', 'custom_text');

Probably needs some tweaks, but what do you think?

#10 @jaredh123
12 years ago

That looks better, but I would ask -- why the resistance to adding an ID or a class?

Here's the reason I don't think this is a great solution: Say there's an instance where a plugin or theme author is using the upload form, but not for images, or not for images that would ever need to have titles or descriptions. With your patch, they could filter the message and return an empty string, but they still end up with a potentially problematic empty paragraph. If the extra paragraph gave them formatting problems, they're still in the same boat of not having an easy way to target that element via CSS.

#11 @scottbasgaard
12 years ago

I just think it should be filterable instead.

The only extra <p></p> tags that are added are if the if() statement returns true and the next needs to be pushed to the next line.

A filter w/ an empty string would literally return nothing.

Make sense?

Update

Well, I guess it would in IE/Opera with files over 100MB.

Last edited 12 years ago by scottbasgaard (previous) (diff)

#12 @jaredh123
12 years ago

Ah, gotcha, my bad. I misread the code, I see now the extra paragraph is only added if the "big-file-warning" is required.

Yeah, in that case, this makes sense, thanks.

#13 in reply to: ↑ 4 @azaozz
12 years ago

  • Keywords close added; has-patch removed

Replying to nacin:

These messages are more or less the equivalent of Help in the uploader iframe. Keeping the user in the dark why he can't upload something and/or not showing the "browser uploader" swith is a bad idea imho. If plugins really want to tweak these, using jQuery to do it is trivial:

jQuery('#file-form > p').not('.submit').hide();

#14 follow-up: @jaredh123
12 years ago

azaozz - I don't think you properly understood the issue.

The point is not to remove the max uploading file size limit message. That seems like it should pretty much always be there.

The point is the "After uploading you may add titles and descriptions" sentence makes no sense in many contexts where a plugin or theme author is using the media upload form.

#15 in reply to: ↑ 14 @azaozz
12 years ago

Replying to jaredh123:

I see :) it makes sense to output that with the post-upload-ui action. Don't think we need new filters or actions there though.

#16 @azaozz
12 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [19384]:

Output the "After a file has been uploaded..." message by using the post-upload-ui action, fixes #18764

#17 @ryan
12 years ago

In [21802]:

Preserve the $format and $link arguments for passing to the previous|next_post_link filter. Pass $post to the filter.

Props Otto42, helenyhou
fixes #19640 #18764

#18 @ryan
12 years ago

Ignore comment:17. Wrong ticket.

Note: See TracTickets for help on using tickets.