Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13306 closed Bug (invalid)

link element in body breaks content

Reported by: Alfonso Martínez de Lizarrondo Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

Go to http://ckeditor.com/demo#full-page as it seems to be the only demo with ACF settings to support <link>

Switch to source mode and in the body (yes, it's not allowed bla bla bla. I'm talking about real users, not experts that read the HTML specifications) insert <link href="/shadowbox/shadowbox.css" rel="stylesheet" type="text/css">

Now go back to design mode and switch to source and you'll find that the body is empty and everything is after it.

Change History (7)

comment:1 Changed 4 years ago by Jakub Ś

Resolution: invalid
Status: newclosed

(yes, it's not allowed bla bla bla. I'm talking about real users, not experts that read the HTML specifications)

The behaviour of editor is, in my opinion, correct. Element which can be used where meta content is expected is being removed from body.

I understand the argument that most users are not experts in HTML but if that is the case they should not work with the source code as if they were. For such case, a plugin should be prepared whcih handles this task for them. Such users should only be required to enter URL to CSS file. The plugin should wrap this URL in link tag and insert it in correct pleace.

Please also note there are many ways in which users can destroy HTML. I'm mean what's the difference between adding <link href="/shadowbox/shadowbox.css" rel="stylesheet" type="text/css"> in a wrong place and e.g. removing accidentally the <body> element or an important <div> in a document with nested <div> elements?

comment:2 Changed 4 years ago by Wiktor Walc

Created an issue that I think handles this case in a more generic way: #13325. I don't think we'll work on this in the nearest future though. It looks more like a task for a 3rd party addon that additionally may re-use some external engine to check and the correctness of the HTML code.

comment:3 in reply to:  2 Changed 4 years ago by Alfonso Martínez de Lizarrondo

Replying to wwalc:

Created an issue that I think handles this case in a more generic way: #13325.

In no way I'm suggesting that the solution for this problem is to remove the inserted code. If the user adds it and the browser allows it (they all do), then it must stay.

comment:4 in reply to:  1 Changed 4 years ago by Alfonso Martínez de Lizarrondo

Replying to j.swiderski:

Please also note there are many ways in which users can destroy HTML. I'm mean what's the difference between adding <link href="/shadowbox/shadowbox.css" rel="stylesheet" type="text/css"> in a wrong place and e.g. removing accidentally the <body> element or an important <div> in a document with nested <div> elements?

The difference is that it's CKEditor the one that it's destroying the contents with its manipulation and processing.

I've added the code using a stylesheet, but as you might know if I used microdata with <link itemprop> then the specs allows the element in the body.

comment:5 Changed 4 years ago by Wiktor Walc

In no way I'm suggesting that the solution for this problem is to remove the inserted code. If the user adds it and the browser allows it (they all do), then it must stay.

I was rather suggesting that the tool should check for potential errors and list them to the end user one by one before the wysiwyg mode is open, leaving the decision to him whether to ignore it or correct it. Something like "The HTML code contains errors, do you really want to switch to WYSIWYG mode?"

comment:6 Changed 4 years ago by Alfonso Martínez de Lizarrondo

But it's not an error, it's only a pedantic error because the user wants to insert a stylesheet just for that page and if CKEditor doesn't modify the content then it works.

I've suggested testing the full-page demo because I think that it's the only one with the ACF disabled and of course in that case it's simpler to add it into the head, but in a normal document I get funnier results comparing git-based CKEditor vs built one. I just thought that the test case should be enough to take a look at it, but it's clear that I'll have to do myself and maybe some years from now you decide that it's time to fix it in the core.

comment:7 Changed 4 years ago by Wiktor Walc

I just thought that the test case should be enough to take a look at it, but it's clear that I'll have to do myself and maybe some years from now you decide that it's time to fix it in the core.

Come on... maintaining a project that is used by so many users and for so many years requires us to fix less, but with strong attention to details and quality. Let's just stop here to avoid a flame as I completely understand that you have rights to have your own, different opinion.


Note that if its about microformats only (and does not affect full page mode), then it can be solved with

config.protectedSource.push( /<link.*?>/gi );

But CKEditor cannot be configured by default this way, because otherwise the CSS stylesheet would not be loaded in the full page mode.

Now getting back to the main issue. I was unable to confirm the reported problem on Firefox and Chrome using CKEditor 4.4.7 (full preset) and the "replacebycode" sample with config.allowedContent = true. I was able to confirm it though on the full page example only. Is it possible that it only affects the full page mode indeed?

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