Duplicate attachment of Wysiwyg behavior with Popups

sirkitree - July 3, 2009 - 00:27
Project:Popups API (Ajax Dialogs)
Version:6.x-2.0-alpha5
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:active
Description

I'm having a problem where tinymce is getting duplicate buttons when using popups api's popups_reference module.

Basically how this works is if you have a content type (say story) with a node reference field to another content type (say photo) you can click a link which becomes attached to the node reference in order to add a new photo node within a popup from the create story page. Looks something like this:

The problem comes into play when both content types have body fields in which they are using wysiwyg api. In the previous image the body fields are the one labeled "TALK ABOUT" and the one within the popup labeled "CAPTION". Since the new content type (photo) within the popup has the same #id on the body field, and popups reapplies all behaviors, the Drupal.wysiwygAttach is called with the same params.field passed into it resulting in a duplication of all buttons in the field.

I suppose that you could form_alter one of the fields to have a different #id to get around this, but I think what would be ideal is if params.field was a bit more specific. I'm not sure if this is possible.

The code to peer at would most likely be within wysiwyg/editors/js/tinymce-3.js line: 47 where Drupal.wysiwyg.editor.attach.tinymce() is defined. You can see that params.field is what is passed to tinymce.Editor() and I'm not sure how to make this more specific or to be able to somehow pass the context along with it.

#1

sun - July 3, 2009 - 01:29
Project:Wysiwyg» Popups API (Ajax Dialogs)
Version:6.x-2.0» 6.x-2.0-alpha5
Category:feature request» task

params.field is, in fact, a CSS ID in all cases. CSS IDs can only exist once on a page. Not sure why TinyMCE is duplicating the toolbar, but, of course, the whole system is going to fail if the same CSS ID exists more than once on a page. Some browsers prefer to use the latest ID, some others keep using the first known ID (DOM element).

The proper fix for this is, and must be, that Popups API never ever renders the same CSS ID more than once on a page.

Aside from that, there's more to properly attaching and especially detaching editors - the primary reason for why we added a "detach" method for JS behaviors in D7. Without that method, most editors won't save the edited content back into the textarea.

Moving to Popups API.

#2

greg.harvey - July 13, 2009 - 18:23

You're lucky! Double buttons would be great! At the moment I get no buttons at all in the popups_reference module's pop-up. Seems to be the same issue. When I switch HTML filter it actually adds and removes buttons on the textarea in the background, presumably because it has the same ID as the ones in the foreground and it the browser (Firefox) arbitrarily picks one.

It never actually adds buttons to the textarea in the pop-up itself. =(

#3

sirkitree - July 13, 2009 - 18:53

Yes, if you notice the screenshot, that is the case here too. The buttons are being duplicated on the original and there is no wysiwyg in the actual popup. And yes, it is because of the duplicate ID.

What needs to be done (and I'm not really sure how feasible this is) is that everything rendered by the popups need to have it's ID changed to something unique by appending to it for something.

This would bring up other issues however, like CSS clashes or misinterpretations. Not sure what the final solution should really be. Seems like a lot of work to go through...

#4

greg.harvey - July 13, 2009 - 19:25

What if we looked at it the other way around? What if the popups module dynamically changed the ID of the textarea on the original form in the background (which would break stuff, but who cares because our focus is in the pop-up, right?) so things behave normally in the pop-up. Then on closing the pop-up we could reinstate the *old* IDs back to the original background textarea, because we know our foreground pop-up form is gone.

Might be easier than trying to generate new IDs for newly opened forms - let them behave as they are and alter the pre-existing IDs to temporarily render them broken.

#5

sirkitree - July 13, 2009 - 20:06

hrm. interesting. I think it sounds plausible, even if it's not ideal. But I'm more interested in practical at this point and getting something working. Launching in two weeks. I'll give this idea a shot and report back my results. Thanks for the idea!

#6

greg.harvey - July 13, 2009 - 21:08

Thank you for taking a look at it! My JavaScript skills are pretty poor (no practice) so while I was willing to have a crack at it, you'd surely get there quicker than I would! Look forward to seeing how you get on. =)

#7

sirkitree - July 13, 2009 - 21:34

So far I've determined this would need to be done within Popups.js.

I changed the #edit-body ID to #edit-body-duplicate within Popups.open(). I then put in code to revert this change within Popups.close().

Results:
Positive - The body within the Popup now has WYSIWYG.
Negative - The WYSIWYG is still duplicated on the original element.

I believe the negative results are happening because of how WYSIWYG's attach function selects what it attaches to. It's not simply looking for #edit-body, it's looking for any field that has the WYSIWYG input filter selected. Somehow we have to make so that this selection criteria is negated.

#8

sun - July 13, 2009 - 21:53

I think you must leave the original IDs intact.

Can't you do something like this in Popups API's callback handler for the newly loaded AJAX content?

var popupID = 123; // ...or however Popups API keeps track of this stuff.
$.get('foo/bar', function (data) { // ...or however Popups API works.
  var $content = $(this); // ...or is it data.result instead of this?

  // Find all DOM elements having a ID attribute.
  $content.find('[id]').each(function () {
    // Append the internal Popups API popup id to the DOM element ID.
    $(this).attr('id', $(this).attr('id') + '-' + popupID);
  });
 
  // Go ahead and insert/inject/replace the popup...
  $content.appendTo(...
});

#9

sirkitree - July 13, 2009 - 22:34

I think a technique like this is worth looking into - however there is still the question of negating Drupal.behaviors.attachWysiwyg() from reattaching itself to the first element.

#10

sirkitree - July 13, 2009 - 23:14

So i got a little of this figured out...

I started editing Popups.openPath() where it defines var ajaxOptions; I added within the success: function(json) { after var inlines = Popups.addJs(json.js);

      // replace all IDs with uniques
      var newId = 0;
      var new_content = '';
      $content = $(json.content);
      $content.find('[id]').each(function () {
        var $this = $(this);
        $this.attr({'id': $this.attr('id') + '-' + newId});
        newId++;
      });
      json.content = $content.html();

This has the desired effect of replacing all of the json.content ID's but causes other errors since scripts like autocomplete.js are looking for the old input ID's and causes errors which don't allow the popups.js to then fully run.

I had a feeling this was going to happen which is why I initially went with greg's suggestion to temporarily adjust the ID behind the popup.

#11

sirkitree - July 13, 2009 - 23:39

Ok, same place in the code - here's a temporary fix JUST TO GET IT WORKING

WARNING: perverse hack forthcoming ---

      // replace all IDs with uniques
      var newId = 0;
      $content = $(json.content);
      $content.find('#edit-body').each(function () {
        var $this = $(this);
        $this.attr({'id': $this.attr('id') + '-' + newId});
      });
      $content.find('.wysiwyg-field-edit-body').removeClass('wysiwyg-field-edit-body').addClass('wysiwyg-field-edit-body-' + newId);
      json.content = $content.html();

#12

sun - July 13, 2009 - 23:54

Renaming the IDs in the "parent window" won't work, because many editors use the ID for internal references. You would get more or less the same as now, but maybe not with all editor behaviors, since not all behaviors of an editor consistently rely on the ID.

Before investigating the server-side generation of IDs (PHP), which will be much harder, hacking the alteration/replacement process might be an option.

Since we know, due to above code, which IDs we replace with new ones, we could also use those string values to replace the corresponding string values in the json.js string, before it is evaluated into real JS.

I.e., move the snippet above var inlines = Popups.addJs(json.js); and perform in .each():

json.js.replace(this.id, newCssId);

Only replacements in the newly loaded Drupal.settings are important to replace.

#13

greg.harvey - July 14, 2009 - 08:31

I'm slightly confused as to why renaming IDs in the "parent window" won't work. Sure, the editor *in the parent window* needs that ID to function, but we're not using it. The parent window is "grayed" out and inactive, so if the editor is temporarily broken who's to care or know? As long as you rename the "parent" IDs back again when the pop-up closes I'm not sure I see the problem? That's probably because I lack some fundamental understanding of how it works tho... =)

#14

sun - July 14, 2009 - 09:25
Title:Duplicate appliction of wysiwyg behavior with Popups Add and Reference» Duplicate attachment of Wysiwyg behavior with Popups

That's because both Wysiwyg module and many editors initialize and attach an editor instance by CSS ID.

In this context, the JavaScript environment is not stateless. Once initialized/attached, a client-side editor won't be attached to an ID that was already initialized/attached. To elaborate:

1. Editor is initialized for #edit-body and attached there.
2. Popups API loads new content, containing new #edit-body.
3. If original #edit-body is renamed only, the JS environment still contains storage/initialization for #edit-body.

If the proposal of #11/#12 works, I'm open to alter Wysiwyg module accordingly to not store/pass wysiwyg settings via CSS classes, but via Drupal.settings. We planned to change that anyway during the rewrite for Wysiwyg 3.x.

#15

greg.harvey - July 14, 2009 - 09:45

Ohhh, storage! I get it. Makes perfect sense. Thanks for taking the time to explain. =)

#16

-Shaman- - August 13, 2009 - 19:24

@sirkitree - I think that You're talking all the time about popups_reference.module not popups_api.module, am I right?

#17

sirkitree - August 13, 2009 - 19:29

-Shaman-
This bug was found through the _use_ of popups_reference, but the core issue of the matter is coming from how Popups API works.

#18

-Shaman- - August 13, 2009 - 19:37

Yup I now that, but which file You modified, I'm talking about this code:

// replace all IDs with uniques
      var newId = 0;
      $content = $(json.content);
...

Is it popups/popups.js?

#19

sirkitree - August 13, 2009 - 19:47

correct.

#20

-Shaman- - August 13, 2009 - 20:10

OK now, Do You know how to make BUEditor appears on popups, or You've been only testing tinymice?

#21

Ghostthinker - August 31, 2009 - 07:53

Hi,
Sorry for pirating this post, but could anyone please explain how to make tinymce via wysiwygapi work? What versions do you use? I make all my popups via hook_popups():

<?php
 
return array(
     
'*' => array(
         
'a[href~=node/add]' => array(
             
'additionalJavascript' => array(
                 
'misc/textarea.js',
                 
'misc/autocomplete.js'
             
)
          ),
)
);
?>

Tank you so much

#22

-Shaman- - September 7, 2009 - 19:05

any thoughts on how to enable bueditor in popups?

 
 

Drupal is a registered trademark of Dries Buytaert.