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

Uncaught TypeError: Cannot read property 'attributes' of null #698

Closed
gfox1984 opened this issue Jul 28, 2017 · 12 comments · Fixed by #5400
Closed

Uncaught TypeError: Cannot read property 'attributes' of null #698

gfox1984 opened this issue Jul 28, 2017 · 12 comments · Fixed by #5400
Assignees
Labels
plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.
Milestone

Comments

@gfox1984
Copy link

Are you reporting a feature or a bug?

Bug

Provide detailed reproduction steps (if any)

In some very specific cases, editable inline widgets will throw an "Uncaught TypeError: Cannot read property 'attributes' of null" after making a modification to the content of the widget.

Please follow the steps below using this JSFiddle:

  1. Select the very last letter of the text inside the widget (= select the "m" of "lorem")
  2. Click the Bold button from the toolbar
  3. Switch to Source mode

Actual result

The following error occurs:

Uncaught TypeError: Cannot read property 'attributes' of null
    at VM232 plugin.js?t=H5SB:23
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.htmlParser.element.forEach (VM226 ckeditor.js:281)
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.htmlParser.element.forEach (VM226 ckeditor.js:281)
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.htmlParser.element.forEach (VM226 ckeditor.js:281)
    at a.<anonymous> (VM232 plugin.js?t=H5SB:23)
    at a.f (VM226 ckeditor.js:11)
    at a.window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.event.CKEDITOR.event.fire (VM226 ckeditor.js:12)
    at a.window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.editor.CKEDITOR.editor.fire (VM226 ckeditor.js:14)
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.htmlDataProcessor.toDataFormat (VM226 ckeditor.js:302)
    at $.getData (VM226 ckeditor.js:1183)

Other details

  • Browser: Chrome 59.0.3071.115 (Official Build) (64-bit)
  • OS: Windows 10 (Creator Update)
  • CKEditor version: 4.7.1
  • Installed CKEditor plugins: "full-all" preset
@msamsel
Copy link
Contributor

msamsel commented Aug 1, 2017

Note:
Error is thrown from here:
https://github.com/ckeditor/ckeditor-dev/blob/dee99e283ce259f8e16f1092e3d7613a5e45a499/plugins/widget/plugin.js#L2726
if ( widgetElement.attributes[ 'data-cke-widget-keep-attr' ] != '1' )

@msamsel msamsel added plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug. labels Aug 1, 2017
@jswiderski jswiderski added the support An issue reported by a commercially licensed client. label Oct 13, 2017
@jswiderski
Copy link
Contributor

The same problem can be reproduced with simplebox widget:

  1. https://sdk.ckeditor.com/samples/simplebox.html
  2. Remove all the content in srouce view and set new content as below
<h1>TEST</h1>
<div class="simplebox">
<h2 class="simplebox-title">Title</h2>
<div class="simplebox-content">
<p>Content</p>
</div>
</div>
<p>TEST</p>
  1. In wysiwyg view, start selection inside h1 - eg. TE^ST to upto end of Content^ inside the simple box
  2. Press delete
  3. Go to source view

@mlewand mlewand added this to the Backlog milestone Oct 13, 2017
@beatadelura beatadelura self-assigned this Oct 16, 2017
@beatadelura
Copy link
Contributor

It seems, this problem is more complex. First of all, I would like to know what is a proper behavior in situations:


Situation A:

  1. Go to https://jsfiddle.net/fvs0k51s/
  2. Select ST Lorem
  3. Press Delete key

Answer:

a) The part of widget ipsum should join to h1 --> <h1>TE ipsum</h1> (Notice that below h1` there is a paragraph with nested widget wrapper

b) The part of widget ipsum should not join to h1 and it should still be a widget

c) The part of widget ipsum should join to h1 but also should still be a widget


Situation B:

  1. Go to https://jsfiddle.net/fvs0k51s/4/
  2. Select last word ipsum
  3. Click Bold

Should be <strong><span>Lorem <strong>ipsum</strong></span></strong> returned?


It looks like inline widgets don't support editables. Follow steps to notice how inline widget vs block widget behaves:

  1. Go to https://jsfiddle.net/fvs0k51s/2/
  2. Select ST Lorem ips
  3. Press Delete key

So it's probably a new Feature Request (check https://dev.ckeditor.com/ticket/10974) rather than a bug. WDYT @mlewand?

@mlewand
Copy link
Contributor

mlewand commented Oct 25, 2017

You're right we do not support inline editables ATM. Can't we patch up this particular exception? Based on what @msamsel said it looks to me that widgetElement is simply null, thus we could add additional check before using / accessing it.

As for cases above I would say that it should not merge widget with the regular content (preceding header), however the (initially) selected content of the widget should get removed. But it's just my on-the-fly opinion, it would need to be discussed with more ppl before we pick anything.

To conclude, @beatadelura can I ask you to:

  • Create a separate feature request issue for inline widget editables, that would link to the Trac ticket you provided.
  • As a fix for this ticket just fix the code in a way that no exception is thrown.

@beatadelura
Copy link
Contributor

New Feature Request added 👍

@beatadelura
Copy link
Contributor

As a fix for this ticket just fix the code in a way that no exception is thrown.

In my opinion, the fix should be done somewhere else because the problem is that additional wrapper is added incorrectly. Please look at this:

  1. Go to https://jsfiddle.net/x7oy3L8f/
  2. Inspect lorem ipsum element
  3. Select ipsum word.
  4. Click Bold

Notice, that additional strong has been created:

<strong><span tabindex="-1" contenteditable="false" data-cke-widget-wrapper="1" data-cke-filter="off" class="cke_widget_wrapper cke_widget_inline cke_widget_spanwidget" data-cke-display-name="span" data-cke-widget-id="0" role="region" aria-label="span widget"><span class="cke_reset cke_widget_drag_handler_container" style="background: url(&quot;https://cdn.ckeditor.com/4.7.3/full-all/plugins/widget/images/handle.png&quot;) rgba(220, 220, 220, 0.5); top: -13px; left: 0px; display: block;"><img class="cke_reset cke_widget_drag_handler" data-cke-widget-drag-handler="1" src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw==" width="15" title="Click and drag to move" height="15" role="presentation" draggable="true"></span></span></strong>

So this should be fixed first.

@mlewand
Copy link
Contributor

mlewand commented May 8, 2018

In my opinion, the fix should be done somewhere else because the problem is that additional wrapper is added incorrectly.

Yes, but that's the scope of #1091 issue. Here we could limit ourself to prevent CKEditor from throwing errors in the console, we don't have to do anything in terms of editables in inline widgets, as we don't support them.

@clarknelson
Copy link

If inline widgets are not supported, why is there an inline widget config option?

@lslowikowska lslowikowska added support:1 An issue reported by a commercially licensed client. and removed support An issue reported by a commercially licensed client. labels Dec 21, 2018
@woody-li
Copy link

Is there any update on it?

@f1ames
Copy link
Contributor

f1ames commented Jun 12, 2020

Hi @woody-li,

unfortunately we don't have any ETA on this issue for now due to other priorities. Do you have any particular case where this issue is visible or is it the same as reported in the issue description?

@lslowikowska lslowikowska added support:2 An issue reported by a commercially licensed client. and removed support:1 An issue reported by a commercially licensed client. labels May 6, 2021
@glen-84
Copy link
Contributor

glen-84 commented Nov 8, 2022

This is still an issue in CKEditor 4.19.1. 😞

I'm not editing the widget at all, just switching between source and WYSIWYG mode.

@glen-84
Copy link
Contributor

glen-84 commented Jan 2, 2023

@Comandeer

Can we patch this by just checking that widgetElement is set?

// If widget did not have data-cke-widget attribute before upcasting remove it.
if ( widgetElement.attributes[ 'data-cke-widget-keep-attr' ] != '1' )
delete widgetElement.attributes[ 'data-widget' ];

And here (element):

// Transfer widget classes from widget element back to data (https://dev.ckeditor.com/ticket/13199).
if ( element.attributes[ 'class' ] ) {
ret.attributes[ 'class' ] = element.attributes[ 'class' ];
}

drupal-ckeditor-libraries-group-machine pushed a commit to drupal-ckeditor-libraries-group/embedsemantic that referenced this issue Jun 1, 2024
drupal-ckeditor-libraries-group-machine pushed a commit to drupal-ckeditor-libraries-group/widget that referenced this issue Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.