Opened 7 years ago

Closed 7 years ago

#10192 closed Bug (fixed)

Closing Lists with Enter does not work with Data Filtering

Reported by: Tobias Hößl Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.1
Component: General Version: 4.1 RC
Keywords: Drupal Cc: wim.leers@…

Description

This problem can be seen in base/samples/datafiltering.html of 4.1RC

When moving the cursor to the numbered list of "Editor 3", the one using "allowedContent" with ul/ol/li enabled, then using the enter key can not be used to close the list anymore. Creating a new item using the enter key still does work. In "Editor 4", the same actions work.

Tested with Firefox and Chrome.

Change History (12)

comment:1 Changed 7 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.1
Status: newconfirmed

Thanks. I confirmed this issue.

comment:2 Changed 7 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:3 Changed 7 years ago by Piotrek Koszuliński

This bug is caused by indent plugin. It allows to do two things: indent/outdent blocks and intend/outdent (nest) lists. However, its state is set based on one requiredContent rule which was created for blocks indentation.

Why does this affect enter key? Because enterkey plugin reuses 'outdent' command which is blocked in 3rd editor.

Therefore two things should be fixed.

  • There is no reason for enter key to reuse indent plugin. This dependency is problematic, because enter key is popular (nearly obligatory plugin) whereas indent is less popular, but now it is required by the first one.
  • However, it won't be a complete fix, because in order to be able to nest lists one would have to allow margin-left style. I can think of two solutions.
    • We can add a second part of requiredContent rule for indent plugin, checking whether lists are allowed. This is a quick fix, but user will be able to indent block when only nesting lists is allowed.
    • Therefore second, but more complex part of this solution would be to make indent and outdent buttons contextual and aware of ACF setting. Command would be refreshed based on current context (if caret is in block or list item) and what is allowed (indenting block and/or list items). This part is pretty tough because there has to be 1:1 relation - if command is enabled and indenting blocks is not allowed, then it should always nest this list item, not indent some block. And vice versa.

Question is - what can we include in 4.1 and what has to wait for... 4.2? I'm for the full solution now, because this thing will be pretty broken with partial approach.

I think that we may for now ignore the enterkey->indent plugins dependency. Especially if fixing it is not only the matter of copying few lines from outdent command to enterkey plugin. However, it would be cool if we will at least take a look on it.

Last edited 7 years ago by Piotrek Koszuliński (previous) (diff)

comment:4 Changed 7 years ago by Tobias Hößl

Is there any way around this, using configuration options, like by enabling margin-left / indent/outdent somehow?

comment:5 in reply to:  4 Changed 7 years ago by Piotrek Koszuliński

Replying to Cato:

Is there any way around this, using configuration options, like by enabling margin-left / indent/outdent somehow?

Yes, you need to allow for 'p{margin-left}' (or paragraph with indent classes if you defined config.indentClasses) - that will enable indent plugin which is required for enter key to work correctly with lists.

However, I'm pretty sure that we'll somehow fix this issue in final release and you won't need to allow margins.

comment:6 Changed 7 years ago by Wim Leers

Cc: wim.leers@… added
Keywords: Drupal added

Related: in Drupal, we don't want the indent/outdent buttons to be usable on anything except for nested list items (ul li & ol li).

comment:7 Changed 7 years ago by Olek Nowodziński

Owner: changed from Piotrek Koszuliński to Olek Nowodziński

comment:8 Changed 7 years ago by Olek Nowodziński

Status: assignedreview

Created branch t/10192 (+tests) with a fix:

  • Allowed filtering rules in form of arrays.
  • Extended requiredContent with li tag.
  • Added indent as a dependency of list plugin.

comment:9 Changed 7 years ago by Olek Nowodziński

Moved further discussion to #10027.

comment:10 Changed 7 years ago by Piotrek Koszuliński

Pushed rebased branch.

comment:11 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_passed

After few more commits (e.g. one fixing tests which were passing on current major ;>) branches are ready. R+.

comment:12 Changed 7 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

Fixed in git:5ed13a3c5001 (dev major) and 32dc39a (tests-v4).

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