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

image2: throws an exception if set a content with an empty figure tag #2506

Closed
vkmet opened this issue Oct 23, 2018 · 2 comments
Closed

image2: throws an exception if set a content with an empty figure tag #2506

vkmet opened this issue Oct 23, 2018 · 2 comments
Assignees
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. plugin:image2 The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Milestone

Comments

@vkmet
Copy link

vkmet commented Oct 23, 2018

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Set extraPlugin as config.extraPlugins = 'image2';
  2. Set a value for ckeditor as <figure class="image"></figure>

image2 plugin throws an exception if ckeditor's content contains empty "figure" tag (or if a "figure" does not contains nor "img" nor "a" tag).

<div id="editor">
    <figure class="image"></figure>
</div>

image2 expects that "figure" must always contains "img" tag or "a" tag that contains "img".
image = el.getFirst( 'img' ) || el.getFirst( 'a' ).getFirst( 'img' );
The issue can be reloved by adding additional checking:
image = el.getFirst( 'img' ) || (el.getFirst( 'a' ) && el.getFirst( 'a' ).getFirst( 'img' ));

Expected result

Exception should not be thrown.

Actual result

Uncaught TypeError: Cannot read property 'getFirst' of null exception is being thrown.

Other details

  • Browser: Google Chrome 70, FireFox 62
  • OS: Windows 10
  • CKEditor version: ckeditor_4.10.1
  • Installed CKEditor plugins: image2
@mlewand mlewand self-assigned this Oct 24, 2018
@mlewand
Copy link
Contributor

mlewand commented Oct 24, 2018

Actually image2 widget should not be upcasted on a figure that does not contain any img elements, as figures are not limited to images.

@mlewand mlewand added type:bug A bug. status:confirmed An issue confirmed by the development team. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. plugin:image2 The plugin which probably causes the issue. labels Oct 24, 2018
@mlewand mlewand removed their assignment Oct 24, 2018
@mlewand mlewand added the target:minor Any docs related issue that can be merged into a master or major branch. label Oct 24, 2018
@engineering-this engineering-this self-assigned this Nov 27, 2018
@Comandeer
Copy link
Member

Merged into 4.11.2.

@Comandeer Comandeer added this to the 4.11.2 milestone Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. plugin:image2 The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants