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

Crash when loading an image caption with a list inside (ListProperties) #13858

Closed
oleq opened this issue Apr 11, 2023 · 0 comments · Fixed by #13859
Closed

Crash when loading an image caption with a list inside (ListProperties) #13858

oleq opened this issue Apr 11, 2023 · 0 comments · Fixed by #13859
Labels
package:list squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Apr 11, 2023

📝 Provide detailed reproduction steps (if any)

  1. Go to https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/features/lists/lists.html#demo-2
  2. Set the following data
<figure class="image"><img src="https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/93c337f3c2eb83f436edba7e778a5983d212dd8ab79859d7.png"  width="1208"><figcaption><ul><li>foo</li></ul></figcaption></figure>

✔️ Expected result

Silence.

❌ Actual result

spector.js:2 Uncaught TypeError: Cannot read properties of null (reading 'start')
    at e.on.priority (snippet.js:4:605515)
    at Ec.fire (snippet.js:4:5541)
    at Ec._convertItem (snippet.js:4:231683)
    at Ec._convertChildren (snippet.js:4:232140)
    at Object.convertChildren (snippet.js:4:230260)
    at Pc.upcastDispatcher.on.priority (snippet.js:4:236015)
    at Ec.fire (snippet.js:4:5541)
    at Ec._convertItem (snippet.js:4:231683)
    at Ec._convertChildren (snippet.js:4:232140)
    at Object.convertChildren (snippet.js:4:230260)

❓ Possible solution

List properties upcast converter tries to upcast attributes despite the "parent" list element not being upcasted in the first place (e.g. disallowed in the image caption). We need to check whether the model range exists first in the converter.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. package:list labels Apr 11, 2023
@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Apr 11, 2023
@mlewand mlewand added the squad:core Issue to be handled by the Core team. label Apr 11, 2023
arkflpc added a commit that referenced this issue Apr 12, 2023
Fix (list): ListPropertiesEditing should not crash if there was a list in an image caption element. Closes #13858.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Apr 12, 2023
@CKEditorBot CKEditorBot added this to the iteration 62 milestone Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:list squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
3 participants