Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Format changes selection when applied to unsupported element #2451

Closed
jswiderski opened this issue Oct 3, 2018 · 1 comment
Closed
Assignees
Labels
status:confirmed An issue confirmed by the development team. support An issue reported by a commercially licensed client. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Milestone

Comments

@jswiderski
Copy link
Contributor

jswiderski commented Oct 3, 2018

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Open nightly sample - http://nightly.ckeditor.com/18-10-02-06-04/full/samples/.
  2. Clear editor contents and type any text.
  3. Select the text and set Formatting to "Heading1" and then apply the Numbered list.
  4. Click on Remove Format button (Notice that Format dropdown no longer shows "Heading 1" at this point)
  5. Change the formatting in the Format dropdown to "Heading2"

Expected result

Format dropdown should display "Heading 2".

Actual result

Format dropdown displays "Format".
Please note however that new Format gets applied properly. Only the selection is being moved from H1 element to its parent ol. If you change the selection and click inside the H1, the Format dropdown will show it correctly again.

Other details

  • Browser: Any
  • OS: Any
  • CKEditor version: 4.0+
  • Installed CKEditor plugins: Format Dropdown.

When Remove Format is applied, it looks like selection is being moved from the actual element to its parent. This behavior is in my opinion correct for inline elements because when format is removed, the inline element (like span or strong) is being removed as well. When it comes to block-level elements, things get more complicated. By default removeFormatTags is only be applied to inline elements and documentation say that only inline elements can be used on that list. There is however nothing stopping from setting this configuration option to config.removeFormatTags = 'h1,div,blockquote'. What is more, such setup will work properly (provided that start of element is selected) when removing formatting form H1 for example (tag will get removed). Since the plugin works ok for both types of tags and the only thing which says otherwise is one sentence in documentation, what should we do?

  1. Apply Remove Format to tag only if it is on the list? That way we could avoid selection changes for not supported tags and leave current behavior untouched. Best solution IMHO.
  2. Block the block-level tags permanently? Disallow them on the list and ignore them when Remove Format is applied?
  3. Ignore it and leave it as it is? Kind of makes sense because this plugin should not be used for not supported tag in the first place but on the other hand how could the user know that.

Screen cast: remove format.zip

@jswiderski jswiderski added type:bug A bug. status:confirmed An issue confirmed by the development team. support An issue reported by a commercially licensed client. labels Oct 3, 2018
@engineering-this engineering-this self-assigned this Nov 27, 2018
@mlewand mlewand changed the title Remove Format changes selection when applied to unsupported element. Remove Format changes selection when applied to unsupported element Dec 7, 2018
@mlewand mlewand added the target:minor Any docs related issue that can be merged into a master or major branch. label Dec 7, 2018
@mlewand mlewand added this to the 4.11.2 milestone Dec 10, 2018
@Comandeer
Copy link
Member

The fix was merged into 4.11.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:confirmed An issue confirmed by the development team. support An issue reported by a commercially licensed client. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants