#22888 closed defect (bug) (fixed)
Block <a> tags are getting stripped from the Editor
Reported by: | georgestephanis | Owned by: | azaozz |
---|---|---|---|
Milestone: | 3.5.1 | Priority: | high |
Severity: | major | Version: | 3.5 |
Component: | Editor | Keywords: | commit |
Focuses: | Cc: |
Description (last modified by )
Steps to duplicate:
- Make a new post.
- Have a single line of text saying 'Foo foo foo'.
- Change it to a h1 block.
- Select the line.
- Add a hyperlink to example.com
- Switch to the Text tab.
- 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)
Change History (28)
#2
@
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
@
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
@
12 years ago
Suggested fix for 3.5.1: just make <a>
only accept inline child elements (like before).
#5
@
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.
#6
@
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.
#8
follow-up:
↓ 9
@
12 years ago
I'm having a couple of different issues with the <a> tag myself.
- Type "Some Text" in the editor of a new post.
- 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:
- "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.
- "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.
- Switching to the text editor shows a couple of line breaks before the closing </a> tag.
- Switching back to the visual editor then shows a paragraph break after the </a> tag.
- 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.
#9
in reply to:
↑ 8
;
follow-up:
↓ 10
@
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
@
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
@
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
@
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:
↓ 16
@
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.
#14
@
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
@
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.
Generally you're specifying the tag that would wrap the selected text or will be inserted at the caret location, together with all attributes.
#16
in reply to:
↑ 13
@
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
@
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.
#18
follow-up:
↓ 19
@
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
@
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
@
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.
Is this a regression in 3.5?
What's stripping it?