Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#22888 closed defect (bug) (fixed)

Block <a> tags are getting stripped from the Editor

Reported by: georgestephanis's profile georgestephanis Owned by: azaozz's profile azaozz
Milestone: 3.5.1 Priority: high
Severity: major Version: 3.5
Component: Editor Keywords: commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Steps to duplicate:

  1. Make a new post.
  2. Have a single line of text saying 'Foo foo foo'.
  3. Change it to a h1 block.
  4. Select the line.
  5. Add a hyperlink to example.com
  6. Switch to the Text tab.
  7. Switch back to the Visual Editor.

THE LINK IS GONE ZOMG WUT?!?!

Basically since we've incorporated the HTML5 Schema for TinyMCE, <a> tags have been upgraded to a block-level element. So TinyMCE will wrap the <h1> tag with the <a> tag -- as opposed to the opposite in 3.4.

Then we go in and strip it out (because reasons).

Let's stop stripping out valid tags that we're adding via the editor in 3.5.1

Attachments (3)

22888.patch (517 bytes) - added by azaozz 12 years ago.
style-formats-test.php (779 bytes) - added by bradyvercher 12 years ago.
Plugin demonstrating style format issue.
22888 - editor_template_src.js.patch (559 bytes) - added by bradyvercher 12 years ago.
Enable editor button for href-less links

Download all attachments as: .zip

Change History (28)

#1 @nacin
12 years ago

Is this a regression in 3.5?

What's stripping it?

#2 @georgestephanis
12 years ago

It's a regression insofar as the new HTML5 schema in TinyMCE is allowing <a> tags to be wrapped around block level tags, like <h1> (which didn't used to happen) -- but the filter that passes through the code when loading it into the visual editor isn't recognizing that and stripping it out (which always used to happen)

So the change from 3.4.2 isn't that we're doing something new in a given situation per se, we're just letting users get into that situation whereas they couldn't previously.

#3 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 3.5.1
  • Priority changed from normal to high
  • Severity changed from normal to major

#4 @markjaquith
12 years ago

Suggested fix for 3.5.1: just make <a> only accept inline child elements (like before).

#5 @azaozz
12 years ago

Yes, caused by setting all block elements (headers, paragraphs, tables, etc.) as valid children of <a>. +1 to @markjaquith's suggestion.

Also as some elements are both phasing and flow, there's no need to repeat them in the "children" strings (B = phasing, C = flow), or perhaps better to set C = all flow elements as per W3C and only use [C] for elements that can contain flow.

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

#6 @azaozz
12 years ago

As far as I see no other elements have changed behavior in TinyMCE. <ins> and <del> still can't contain blocks but that's the same as in 3.4.

#7 @SergeyBiryukov
12 years ago

  • Description modified (diff)

#8 follow-up: @bradyvercher
12 years ago

I'm having a couple of different issues with the <a> tag myself.

  1. Type "Some Text" in the editor of a new post.
  2. Highlight the text and click the "Insert link" button. Enter a URL and click the "Add Link" button.

At this point, I've seen a couple of different behaviors:

  1. "Some Text" will become a link, but the cursor is inserted before the text and attempting to type any additional text includes it in the <a> tag, even when adding paragraph breaks.
  2. "Some Text" becomes a link, but the cursor appears to be floating in the center of the editor and halfway under the toolbar. Adding additional text inserts a paragraph break, but otherwise appears to work fine.

However, in both cases, switching the editor mode causes issues.

  1. Switching to the text editor shows a couple of line breaks before the closing </a> tag.
  2. Switching back to the visual editor then shows a paragraph break after the </a> tag.
  3. And switching back to the text editor again, shows a non-breaking space.

There doesn't seem to be any consistency and it's creating all sorts of formatting issues.

In addition to that, custom style formats that do nothing more than add a class to the <a> tag don't work. Just to demonstrate, insert <a class="btn">Button</a> into the text editor, then switch to visual mode. The "Insert link" button is depressed, but disabled and the "Unlink" button doesn't actually do anything.

@azaozz
12 years ago

#9 in reply to: ↑ 8 ; follow-up: @azaozz
12 years ago

Replying to bradyvercher:

All of these problems are fixed after reverting back to not allowing block elements in <a>. The change is really small, attached a patch to show exactly where it is (you will need SCRIPT_DEBUG to test it).

#10 in reply to: ↑ 9 @bradyvercher
12 years ago

  • Cc brady@… added

Replying to azaozz:

Thanks! That did fix most of the issues.

The only other thing is the "Insert link" button being disabled if there isn't an href attribute on an <a> tag.

#11 @azaozz
12 years ago

Technically an <a> tag without href (and with name) is an anchor. TinyMCE shows these as little yellow anchor images. It is possible to make an anchor into a link as well although this is outdated/deprecated.

#12 @bradyvercher
12 years ago

Right, and that works fine, but if the name, id, and href attribute are all missing, the "Insert link" button is depressed and disabled and a link can't be added using TinyMCE.

The use case here is when the style format dropdown is enabled, it can be used to apply a class to an <a> tag. The problem arises if a user applies the style before adding a link, the href attribute won't exist, so they can't add the link. I guess as long as there's workaround (link before style), it could just be a training issue.

#13 follow-up: @azaozz
12 years ago

Yeah, that is a quirk. But if there is an <a> without name or href it is completely invalid html. Don't think this can be inserted using TinyMCE, the only way would be if the user typed it in the Text editor.

Testing this further: when the user applies a style to some selected text, TinyMCE inserts a span around it and a link can be added.

@bradyvercher
12 years ago

Plugin demonstrating style format issue.

#14 @bradyvercher
12 years ago

Here's a plugin demonstrating how an <a> tag without an href attribute can be inserted. If there's not an issue with my code and it can't be fixed, I'll just make sure the style and link are added in the the order that works.

#15 @azaozz
12 years ago

Yes, using style_formats in the TinyMCE init overwrites the internal handling of the elements. The syntax for this option is almost the same as for the 'formats' option which is used for replacing the default behavior in TinyMCE for different controls and elements, so it needs the href attribute there too.

Version 0, edited 12 years ago by azaozz (next)

#16 in reply to: ↑ 13 @toscho
12 years ago

  • Cc info@… added

Replying to azaozz:

But if there is an <a> without name or href it is completely invalid html.

No, that’s valid. There are even use cases like deactivating links to the current page in a menu without affecting the element names. But in a post content it is indeed probably useless.

Just nitpicking. :)

#17 @azaozz
12 years ago

Right, <a> in HTML5 is quite different than in HTML4/XHTML1. TinyMCE doesn't yet support all new user cases for it (like block children, the original problem in this ticket). Another change is that <a> is never an anchor any more, always a link.

@bradyvercher
12 years ago

Enable editor button for href-less links

#18 follow-up: @bradyvercher
12 years ago

The problem I'm having is that by specifying the href attribute in the style format, it will overwrite an existing href attribute in the editor when the only thing that needs to be done is add a class. I don't want to define a wrapper, but rather merge the attributes, which would seem to indicate that I shouldn't define an href in the style format. The only problem is the style format will then add the <a> tag without an href and the "Insert/edit link" button will then be disabled.

In any case, I dug through the code and found where this is being handled. From the discussion so far, it sounds like the only reason the button is being disabled is to account for named anchors. If so, are there any downsides to making the change in the patch so href-less <a> tags can be treated like links?

#19 in reply to: ↑ 18 @azaozz
12 years ago

Replying to bradyvercher:

...are there any downsides to making the change in the patch so href-less <a> tags can be treated like links?

Ah, I see. That's a good question. I'm afraid this will need to be changed upstream. Thinking the Link/Unlink buttons shouldn't be disabled when the user has clicked in an <a> tag, no matter if there's an href, name, or no attributes. That would change the way the Anchor button works (this button is not used in WordPress by default) but would fit the HTML 5.0 spec better.

#20 @markjaquith
12 years ago

  • Keywords commit added; needs-patch removed

Plan of action: 22888.patch for 3.5.1 — but also wait for a better upstream fix.

#21 @ruud@…
12 years ago

  • Cc ruud@… added

#22 @azaozz
12 years ago

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

In 23218:

TinyMCE: prevent <a> from wrapping block elements, fixes #22888 for 3.5

#23 @azaozz
12 years ago

In 23219:

TinyMCE: prevent <a> from wrapping block elements, fixes #22888 for trunk

#24 @azaozz
12 years ago

#23114 was marked as a duplicate.

#25 @azaozz
12 years ago

#23139 was marked as a duplicate.

Note: See TracTickets for help on using tickets.