Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13254 closed Bug (fixed)

Cannot outdent block after indent when using divarea plugin

Reported by: Jonathan Owned by:
Priority: Normal Milestone: CKEditor 4.4.8
Component: General Version: 4.4.6
Keywords: Cc:

Description

I'm using the divarea plugin, rather than an iframe, which is important to this issue.

When I indent a block (not a list) in the editor, the outdent button remains disabled and I'm not able to outdent it again. I can continue to indent to additional levels, but can never outdent.

Here's the simplified HTML to reproduce the problem.

<html>
<head>
    <script src="ckeditor/ckeditor.js"></script>
</head>
<body>
    <div>
        <ul>
            <li>
                This is just a list item. But CKEditor is below.<br>

                <textarea id="something">some text</textarea>

                <script>
                    CKEDITOR.replace("something");
                </script>
            </li>
        </ul>
    </div>
</body>
</html>

It turns out the state of the outdent button/command is being set to TRISTATE_DISABLED via the return from refresh on lines 126-127 in my version of plugins/indentblock/plugin.js, when getIndent( firstBlock ) returns NaN. This is because firstBlock is the wrong element--one completely outside the editor element!

The root of the problem is on lines 58-60 of plugins/indentblock/plugin.js:

if ( !firstBlock.is( $listItem ) ) {
	firstBlock = firstBlock.getAscendant( $listItem ) || firstBlock;
}

This use of getAscendent backs all the way out of the editor container and up to the first list item on the page. Due to the use of list items in our responsive design layout, we do, indeed, put the editor inside an <li>, as you can see in the HTML above. This problem obviously wouldn't happen when using an iframe, since getAscendent would be confined to that iframe.

I've been able to fix (or at least work around the issue) by changing replacing lines 58-60 with:

if ( !firstBlock.is( $listItem ) ) {
	var query = function ( element ) {
		return element.nodeName in $listItem && path.contains( element );
	};

	firstBlock = firstBlock.getAscendant( query ) || firstBlock;
}

Probably not applicable to this issue, but I'm using:

  • OS: Fedora 21
  • Browser: Chrome 40.0.2214.93 (64-bit)

Attached is my build-config.js file.

Attachments (1)

build-config.js (1.6 KB) - added by Jonathan 4 years ago.
Custom CKEditor build configuration

Download all attachments as: .zip

Change History (5)

Changed 4 years ago by Jonathan

Attachment: build-config.js added

Custom CKEditor build configuration

comment:1 Changed 4 years ago by Jonathan

My initial attempt to fix the issue caused problems with indented lists. However, I forked the CKEditor repo and got the Bender tests working. Then I reproduced the problem with a test and fixed it. The following seems to work nicely:

if ( !firstBlock.is( $listItem ) ) {
	var ascendant = firstBlock.getAscendant( $listItem );

	firstBlock = ( ascendant && path.contains( ascendant ) ) || firstBlock;
}

All unit tests pass (except MathJax, which I skipped because I didn't download and configure it). I've created a PR with the new test and fix: https://github.com/ckeditor/ckeditor-dev/pull/183

Thanks!

comment:2 Changed 4 years ago by Jakub Ś

Keywords: divarea indent outdent indentblock removed
Status: newconfirmed
Version: 4.4.74.4.6

I have been able to reproduce this problem from CKEditor 4.4.6 with divarea editor put inside list. I'm not sure however if this is correct version. Here is why (please use attached sample):

  1. The indent button is enabled, after clicking into editor, starting from version 4.4.6
  2. Both buttons were disabled from 4.2 to 4.4.5
  3. Before version 4.2 indenting editor content indented outer list.

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

Resolution: fixed
Status: confirmedclosed

Fixed on master with git:d63c062. Thanks @jcttrll!

comment:4 Changed 4 years ago by Piotrek Koszuliński

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