Opened 3 years ago

Closed 18 months ago

Last modified 16 months ago

#13585 closed Bug (fixed)

Cannot read property 'equals' of null when trying to wrap div elements in another div.

Reported by: Wiktor Walc Owned by: Tomasz Jakut
Priority: Nice to have (we want to work on it) Milestone: CKEditor 4.7.0
Component: General Version: 4.0 Beta
Keywords: Cc:

Description

Found in Google Chrome.

  1. Open http://ckeditor.com/demo#full
  2. Start with two div elements with selection that contains a part of first and the second div:
<div  class="para">
     Some [content
</div>
<div class="note">
     This is] a note
</div>
  1. Press Div button and attempt to insert a div.

Result:

Uncaught TypeError: Cannot read property 'equals' of null
createDiv @ div.js:147

Change History (22)

comment:1 Changed 3 years ago by Wiktor Walc

Status: newconfirmed

comment:2 Changed 3 years ago by Wiktor Walc

Note: did not check versions older than 4.0.

comment:3 Changed 3 years ago by Jakub Ś

Version: 4.04.0 Beta

Problem can be reproduced from CKEditor 4.0 beta. In CKEditor 3.6.x editor, divs are wrapped in div.

comment:4 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5

comment:5 Changed 3 years ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:6 Changed 3 years ago by Tomasz Jakut

Status: assignedreview

Pushed fix to branch:t/13585.

comment:7 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:8 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:9 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:10 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:11 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:12 Changed 2 years ago by Tade0

Status: reviewreview_failed

I managed to break something eventually. Here's how you can do the same(in Chrome):

  1. Go to: http://tests.ckeditor.dev:1030/tests/plugins/div/manual/wrap2divsindiv
  2. Select the text inside the square brackets.
  3. Use the div button to insert a <div>.
  4. Repeat 3. on the same selection.

The result is an error: node.js:270 Uncaught TypeError: Cannot read property 'insertBefore' of null in file core/dom/node.js

comment:13 Changed 2 years ago by Tomasz Jakut

Status: review_failedreview

The issue was caused by the fact that sometimes groupByDivLimit returned nodes which weren't inside DOM (they didn't have parents). I added proper checks in code & pushed changes to branch:t/13585.

comment:14 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:15 Changed 2 years ago by Tade0

Status: reviewreview_failed

We're going to need a unit test that specifically tests this scenario.

For now in the unit tests blockGroups[ i ].length is never zero, so this path is not covered.

comment:16 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:17 Changed 21 months ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.

comment:18 Changed 20 months ago by Marek Lewandowski

Milestone: CKEditor 4.6.2
Priority: NormalNice to have (we want to work on it)

*t.jakut* won't be able to work on it in upcoming weeks. Let's move it into the backlog.

comment:19 Changed 19 months ago by Tomasz Jakut

Status: review_failedreview

I've added proper unit test.

comment:20 Changed 18 months ago by Tade0

The test is indeed proper.

Fixed with git:191a00ebe998dda70b2a6fb248f65b4647b69268.

comment:21 Changed 18 months ago by Tade0

Resolution: fixed
Status: reviewclosed

comment:22 Changed 16 months ago by Marek Lewandowski

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