When media_youtube is enabled, users can't choose the media format (Link, Preview, Small, Large, Original, etc) when embedding video in WYSIWYG textareas using the media filter. Is there a reason why users should not be able to choose the size of their embedded YouTube videos? It seems like the 'autosubmit' behavior was intentionally enabled, but it's not clear why.

I would prefer that users have the opportunity to choose the media format, so I've attached a patch that removes the autosubmit behavior from the format dialog.

Comments

ar-jan’s picture

Priority: Normal » Major

Is the autosubmit behavior defined only in code? I'm not seeing any setting for this in the UI.
Being able to select a media format for display, like with images, is quite important imo.

stijndm’s picture

I had a look at this yesterday. It's hardcoded in the module. I did a quick test by removing the code. I would then get format selection form but, some formats won't submit (javascript errors: missing attributes, meaning nothing is set for that format). Those formats that would submit only showed an icon or a link. So I'm guessing that is the reason for the autosubmit.

Good to know: I was also using the media_vimeo module (which has the same code, which I also removed) and the mediaelement module for videos.

Some observations:
- Submit failure (javascript errors) seemed to occur on displays that had the youtube, vimeo, mediaelement formatters enabled.
- If I used one of the other displays it would insert an html link or image instead of a media token with formatter settings.
- If I would add a video formatter to one of the 'working' displays it would become unusable in the format selection (javascript errors)
- some other stuff I can't seem to remember

So it seems there is quite a lot to look at before we can remove the autosubmit code.

victoriachan’s picture

Hi,

Can someone attach the patch please? Did becw forget to attach the patch? I can't seem to see it on this page.

Thanks,
Victoria

jkaine’s picture

I ran into the same issue, and on a site with a tight deadline. Ended up using CSS styles to reset the width and height-- via the CKeditor styles menu.

Running short on time here. The gist of the CSS controls can be found here: http://drupal.org/node/1244372

If any of you want more on this, let me know.

andrewbelcher’s picture

Component: User interface » Code
Priority: Major » Critical
StatusFileSize
new511 bytes

I would say this is critical as it renders one of the key features of core media un-usable. I've created an issue in the media_vimeo queue with a patch, and here is a patch for media_youtube...

stijndm’s picture

I believe the submitted patch in #5 is insufficient. I tried this a couple of months back (#2). The only thing the patch did for me was preventing the autosubmit. The steps afterwards, and properly inserting and rendering the video are then broken.

(This was together with the media_vimeo and mediaelement projects. And heavily configured media diplays.)

andrewbelcher’s picture

After applying the patch above, I seemed to be able to insert things properly. It might be that you're experiencing a related issue which is that if the code that get's rendered doesn't include an img tag (for example if you only have the embed as an rendering method in file display), then WYSIWYG cannot handle it... I think that is what #1062948: Issue getting Media, Styles and WYSIWYG working together with MediaElement.js is about... At least in part (comment 6 was the issue I had).

stijndm’s picture

Allright, thanks for the heads up on the issues with MediaElement. I always assumed it was a media_youtube/vimeo issue because submitting the worked when I selected a custom uploaded video (although formatting was wrong).

I'll keep an eye on the related issues, see where this goes. When I have time, I'll try this patch on a vanilla media + media_youtube install, see what happens.

RobW’s picture

Status: Active » Postponed

I believe the reason for the autosubmit is to prevent problems with wysiwyg embedding described in #1283844: [meta] Improve WYSIWYG integration. I think we have to postpone this until the Media issue is resolved.

lucascaro’s picture

In case it helps anyone, here's the patch re-rolled against the current -dev.

I am getting the following errors though:

Warning: array_merge() [function.array-merge]: Argument #2 is not an array in media_youtube_preprocess_media_youtube_video() (line 98 of modules/contrib/media_youtube/includes/themes/media_youtube.theme.inc).
Warning: implode() [function.implode]: Invalid arguments passed in template_process() (line 2404 of includes/theme.inc).

That error is due to $variables['options']['attributes']['class'] being = to "media-image" and not an array.

In the following line:

    $variables['classes_array'] = array_merge($variables['classes_array'], $variables['options']['attributes']['class']);

That could be changed to

    $variables['classes_array'][] = $variables['options']['attributes']['class'];

and that removes the error, but a safer option would be

  if (isset($variables['options']['attributes']['class'])) {
    if (is_array($variables['options']['attributes']['class'])) {
      $variables['classes_array'] = array_merge($variables['classes_array'], $variables['options']['attributes']['class']);
    }
    else {
      $variables['classes_array'][] = $variables['options']['attributes']['class'];
    }
  }

What do you think?

The second patch (media_youtube-fix_autosubmit-1240976-10-b.patch) contains both changes mentioned above and seems to work on my development site.

RobW’s picture

Thanks for the re-roll, lucascaro. There's an issue for those errors at #1797668: Errors when embedding youtube video with media_youtube & tinymce 'media' plugin; your last snippet is exactly the code I suggested there, just haven't had time to test before committing.

RobW’s picture

Status: Postponed » Active

Let's give this some testing.

lucascaro’s picture

FWIW I'm using it and so far there seem to be no problems with the patch. Will report back if I find anything. :)

RobW’s picture

Title: show format settings dialog from appearing when embedding via wysiwyg » Allow users embedding videos with the insert Media wysiwyg button to pick display format
StatusFileSize
new669 bytes

Since the default for autosubmit is false, I believe we can take the whole hook out.

lucascaro’s picture

+1
With #1797668: Errors when embedding youtube video with media_youtube & tinymce 'media' plugin committed, #14 is the only thing missing to get this working :)

lucascaro’s picture

Status: Active » Reviewed & tested by the community

also confirming that appliyig this patch to f2a44ea works as expected.

RobW’s picture

Status: Reviewed & tested by the community » Fixed

Committed: http://drupalcode.org/project/media_youtube.git/commit/825d80b. Thanks to everyone who contributed and Lucas in particular for the cross post debugging and speedy testing.

Status: Fixed » Closed (fixed)

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