Problem/Motivation

When using Chrome (but not Firefox - I haven't tested on Safari nor IE) media images added via the WYSIWYG do not link properly when adding a link via LinkIt.

Steps to reproduce:

  • Use Chrome
  • Fire up a fresh install of Panopoly
  • Go to node/add/panopoly-page
  • Add some dummy text and use the "Media" button to add a new image - set it to "Display as" "Quarter size".
  • Use the LinkIt button to link it to https:www.drupal.org
  • Note how all the text in the WYWISYG is now linked.

Proposed resolution

No idea. I am so tired of WYSIWYG and their associated issues. Can we outlaw them?

Remaining tasks

All the difficult stuff.

User interface changes

Completely remove WYSIWYG from the internet?

API changes

n/a

Data model changes

n/a

Comments

ultimike created an issue.

cboyden’s picture

@ultimike I am able to reproduce this in certain situations on Chrome, but not others. Updated steps to reproduce:

  1. Do a clean install of the latest Panopoly with demo content.
  2. Log in as an administrator, using the Chrome browser.
  3. Create a new node of type Panopoly Page.
  4. In the WYSIWYG, type in a paragraph of text such as "Body text."
  5. Place the cursor at the beginning of the paragraph.
  6. Insert an image using the Media Browser.
  7. Select the image.
  8. Using the Left or Right align button, align the image.
  9. Select the image again.
  10. Using the Linkit button, add a link to the Great Vegetables page.
  11. Note that when the Linkit dialog is closed, both the image and the text are linked.

The key is the alignment. I was not able to reproduce the problem when the image was not aligned.

dsnopek’s picture

I looked into this pretty extensively for UC Berkeley. I ultimately came to the conclusion that this would be a very difficult problem to solve, which is quite likely beyond my abilities. Also, it appears to be a bug in TinyMCE 3, which is an obsolete WYSIWYG editor (replaced by TinyMCE 4) and we're going to be switching to CKEditor 4 in Drupal 8 anyway.

As Panopoly maintainer, my inclination would be to punt on it (and other non-trivial WYSIWYG issues) until Drupal 8, because even if we could fix it, what's the point of fixing TinyMCE 3. If we're gonna be fixing mindbendingly difficult problems, I'd rather be doing it for current libraries. :-)

In any case, I'm going to copy my notes from the private ticket in case someone else smarter and more motivated wishes to actually take a stab at this!


This is so weird! In messing with this a little bit, I could only get it to happen when the <img> tag came at the beginning of the <h3> before the text. If I move the image to the end of the <h3> after the text, then it worked as expected.

I'll keep poking!


I Googled for a bit to see if I could find anyone else talking about this issue - didn't find anything.

Digging into the code: the outer API that gets called for this is mceInsertLink which leads to tinymce.Formatter.apply() which is insanely complicated. I've been trying to step through it in the debugger but haven't yet learned more about what's going wrong here..


Some notes for myself as I try to understand this...

This code appears to be what starts to expand the selection beyond the image:

            // Move start/end point up the tree if the leaves are sharp and if we are in different containers
            // Example * becomes !: !<p><b><i>*text</i><i>text*</i></b></p>!
            // This will reduce the number of wrapper elements that needs to be created
            // Move start point up the tree
            if (format[0].inline || format[0].block_expand) {
                if (!format[0].inline || (startContainer.nodeType != 3 || startOffset === 0)) {
                    startContainer = findParentContainer(true);
                }

                if (!format[0].inline || (endContainer.nodeType != 3 || endOffset === endContainer.nodeValue.length)) {
                    endContainer = findParentContainer();
                }
            }

Before this runs, both startContainer and endContainer point at the img DOM node, but after this, the startContainer points at the h3 DOM node. Strangely the endContainer is still the image, which is a sort of nonsensicle range. I haven't stepped further into the code yet, but I imagine the code later will try to rectify that.


Several lines later there's this:

            // Setup index for startContainer
            if (startContainer.nodeType == 1) {
                startOffset = nodeIndex(startContainer);
                startContainer = startContainer.parentNode;
            }

... which is where the startContainer ends up pointing at the body node


And then this:

			// Setup index for endContainer
			if (endContainer.nodeType == 1) {
				endOffset = nodeIndex(endContainer) + 1;
				endContainer = endContainer.parentNode;
			}

Which sets the endContainer to the strong tag.

This range is eventually what gets returned, which is totally non-sensicle, so something later must be expanding the end to the whole document too, but it's not in the expandRng() method which is what I've been walking through.


Based on what I've been learning so far:

  • This is dependent on the img being the first child of it's parent - if I move it to the last child, the problem doesn't happen
  • This definitely seems like a TinyMCE bug. It's using this expandRng() internal method to try and expand the selection to wrap the most logical chunk of the DOM tree in order to reduce the necessary DOM modfications and it's getting it really, really wrong
  • The amount of code involved in applying the link (including the expandRng() stuff) is just massive and difficult to understand. I'm not super confident that I could modify it and not break something else, given my level of understanding of this code

So, I'm not super sure what the solution should be. We're already on the latest TinyMCE in the 3.x branch and I couldn't find any issues or patches out there about this. Updating to TinyMCE 4.x is a whole other can of worms, and really, that's not super worth it when we'll be going to CKEditor 4 in Drupal 8 which is way newer and actively maintained (so theoretically better) but also having a totally different set of bugs.

My vote would be for documenting manual workarounds for difficult to solve WYSIWYG issues like this and moving on, since fixing this would be super hard and the effort won't carry so far into the future.