Opened 9 years ago

Last modified 9 years ago

#13802 confirmed Bug

Whitespace removed for button tag

Reported by: Thomas Shafer Owned by:
Priority: Normal Milestone:
Component: General Version: 3.0
Keywords: Cc:

Description

Steps to reproduce

  1. Load this html
<!DOCTYPE html>
<html>
<head>
  <title>Whitespace button test</title>
  <script type="text/javascript" src="https://cdn.ckeditor.com/4.5.4/standard-all/ckeditor.js"></script>
</head>
<body>
  <div id="editor">
    <p>Hello <strong>Name</strong>,</p>
    <p>Hello <button>Name</button>,</p>
  </div>
  <script type="text/javascript">

    var element = document.getElementById('editor');

    var options = {
      extraAllowedContent: 'button'
    };

    var editor = CKEDITOR.replace(element, options);

  </script>
</body>
</html>

Expected result

Space should be preserved between Hello and <button> tags.

Actual result

Space is removed between Hello and <button> tags.

Other details (browser, OS, CKEditor version, installed plugins)

Tested in Chrome on Mac OSX.

Change History (6)

comment:1 Changed 9 years ago by Thomas Shafer

I included <p>Hello <strong>Name</strong>,</p> in the example to demonstrate how the expected functionality works for <strong> elements but not <button> elements.

comment:2 Changed 9 years ago by Thomas Shafer

Ok I have found the issue. The issue is that https://github.com/ckeditor/ckeditor-dev/blob/master/core/htmlparser/element.js#L46 says the element is blockLike specified here: https://github.com/ckeditor/ckeditor-dev/blob/master/core/dtd.js#L300. So it removes the whitespace. Removing button: 1 from the list fixes the issue.

comment:3 Changed 9 years ago by Thomas Shafer

I created a pull request for this: https://github.com/ckeditor/ckeditor-dev/pull/219 . I don't think the PR is necessarily the right answer, but it shows why the error occures.

Version 1, edited 9 years ago by Thomas Shafer (previous) (next) (diff)

comment:4 Changed 9 years ago by Jakub Ś

Status: newconfirmed
Version: 4.5.43.0

The button is phrasing content http://www.w3.org/TR/html-markup/button.button.html#button.button so the issue seems valid but PR not necssairly.

comment:5 Changed 9 years ago by Thomas Shafer

hey @j.swiderski, thanks for confirming. The PR is mostly a place that I found to make the change, but not necessarily the correct code change.

I'm solving this in my own app with delete CKEDITOR.dtd.$nonEditable.button; after loading CKEditor.

comment:6 Changed 9 years ago by Thomas Shafer

I added a broken test addressing the issue: https://github.com/ckeditor/ckeditor-dev/pull/226

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy