Opened 12 years ago

Closed 12 years ago

Last modified 6 years ago

#2253 closed Bug (wontfix)

Empty anchor tags being removed.

Reported by: Jeffrey Hundstad Owned by:
Priority: Normal Milestone:
Component: General Version: FCKeditor 2.3.2
Keywords: Confirmed HasPatch Cc:

Description

If you make the code <a href="bob"></a> FCKeditor will eat it at it's first opportunity. Since this is perfectly valid code I'm pretty sure this was an introduced bug while trying to fix SF-BUG 1556878. They tried to fix the outcome not the cause and then removed perfectly valid xhtml in the process.

The easiest way to reproduce this is as follows:

  1. Start an FCKEditor.
  2. Go to source mode (in FCKEditor)
  3. Type in <a href="bob"></a> (note nothing in the contents area of the anchor)
  4. Go to WYSIWYG mode.
  5. Go back to source mode.

You'll find that your typing has vanished.

Here's the fix. The source file is editor/_source/internals/fckxhtml.js in function FCKXHtml.TagProcessors .

If you remove the following lines the "expected" behavior returns.

                // Firefox may create empty tags when deleting the selection in some special cases (SF-BUG 1556878).
                if ( htmlNode.innerHTML.Trim().length == 0 && !htmlNode.name )
                        return false ;

The real code that gets run is editor/js/fckeditorcode_gecko.js and editor/js/fckeditorcode_ie.js .

It's a little tricky to edit this because the lines are huge. Remove the following text from both files and you'll be golden:

if (B.innerHTML.Trim().length==0&&!B.name) return false;

I suggest this code somehow makes it into the codebase as a fix.

--

Jeffrey Hundstad

Change History (8)

comment:1 Changed 12 years ago by Wojciech Olchawa

Keywords: Confirmed HasPatch added
Version: SVN

comment:2 Changed 12 years ago by Alfonso Martínez de Lizarrondo

Summary: [FIXED] Empty anchor tags being removed.Empty anchor tags being removed.
Version: SVNFCKeditor 2.3.2

<a href="bob"></a> is not an anchor, it is a empty link, so I don't know how that can be useful for the users, they can't click on it, you can't use it to mark a destination, they won't even know that it is there!

So applying your patch just reverts the fix for SF-1556878 and then we get back the old bug.

comment:3 in reply to:  2 Changed 12 years ago by Jeffrey Hundstad

This was actually breaking working code (xhtml) in our real environment. What we had was:

<a class="interdoc" href="#testing" id="testing"></a> and then used CSS to mark that so it has a graphical marker as a background that indicates an inter-document link.

I'd submit while at first it looks useless it is valid xhtml1.1 and this is something that CSS gurus use all the time to add accessibility attributes for non-sighted readers.

comment:4 Changed 12 years ago by Frederico Caldeira Knabben

Resolution: wontfix
Status: newclosed

Most of the time a <a href="..."></a> is unwanted and some operations in the editor may leave those kids of tags laying around. They are not visible to the user using the editor, so it is safe to just ignore them on output.

Yours of course is a special case. My recommendation is appending also the "name" attribute to your tags, so they get preserved: <a href="..." name="..."></a>.

I'm closing it as "wontfix", but it is almost a CantFix actually.

comment:5 in reply to:  4 Changed 12 years ago by Jeffrey Hundstad

Any way I can convince you to make a configuration option for this? Like the FCKConfig.IgnoreEmptyParagraphValue option? IMHO, it seems that breaking valid code is not necessarily the best direction for the editor.

comment:6 Changed 12 years ago by Frederico Caldeira Knabben

Valid code (the code that passes W3C validation), in any way means good code. It is a step to it, but the validator is not able to understand how meaningful that code is. The HTML 5 standards are being an evolution in this sense, where strict rules are mixed with good practice recommendations.

In the case of <a>, the HTML 5 specifies that its contents must be "Phrasing content". Let me that the specific text about it:

"As a general rule, elements whose content model allows any phrasing content should have either at least one descendant text node that is not inter-element whitespace, or at least one descendant element node that is embedded content."

Leaving it configurable would be an "anti-feature". It would bring some known bugs back, and you would end up with undesired <a> elements, other than those that you really want to have.

comment:7 Changed 12 years ago by Jeffrey Hundstad

You write:

My recommendation is appending also the "name" attribute to your tags, so they get preserved: <a href="..." name="..."></a>.

I'd actually be ok with this per se, but that code will not validate as html-1.1 which is a requirement for us. Is it possible to have it work with any attribute including id like follows: <a id="myid" href="#here"></a>

I can totally understand the removal of: <a></a> as this is pretty much totally useless... but with an id or class or style it feels like it would be useful even with the W3C paragraph you listed. Comments????

comment:8 Changed 6 years ago by larrykl

How will this not be addressed? It is totally valid to have an empty anchor tag. You can have an empty anchor tag with CSS that sets a height and width and background image and you've got a clickable image link! But CKEditor is stripping this out? That is a bit invasive for a WYSIWYG editor with a source mode.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy