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

The scrollIntoView() method uses non-existent "selection.type" property #4041

Closed
zentby opened this issue May 9, 2020 · 6 comments
Closed
Assignees
Labels
core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. type:bug A bug.
Milestone

Comments

@zentby
Copy link

zentby commented May 9, 2020

if ( this.type != CKEDITOR.SELECTION_NONE )

this.type in the code above is always "undefined" as the type is not being maintained anywhere else. The correct reference call should be this.getType().

@f1ames
Copy link
Contributor

f1ames commented May 11, 2020

Hello @zentby,

how did you find this issue, observed somewhere "in the wild"? This would mean that scrollIntoView() also executes when there is no selection in the editor probably throwing an error :thinking:

@zentby
Copy link
Author

zentby commented May 11, 2020

Hi @f1ames ,

I ran into this issue with my customized plugin that integrates with "autocomplete". The error is thrown within the execution of the next line, this.getRanges() is an empty list. The higher call stack is the "autocomplete"'s commit call. I marked a breakpoint here and observed the value is always undefined. The issue happens in Safari only.

@f1ames
Copy link
Contributor

f1ames commented May 20, 2020

Thanks for the clarification @zentby 👍 Would yo be able to prepare a simple demo which reproduces this issue (e.g. on Codepen - you can start by forking this pen)?

@zentby
Copy link
Author

zentby commented May 22, 2020

Try this one
The issue could be observed in Safari (Mac) console. The editor didn't crash in the pen because I couldn't easily get a minimal example from my plugin.

If you set a breakpoint at LN:10574 as shown below, you'll be able to see that the this.type is always undefined.

image

Reproduce step:

  1. Click the plugin button on the toolbar

@f1ames
Copy link
Contributor

f1ames commented May 25, 2020

Thanks for the demo @zentby. There is something strange going on - from what I have checked our CKEDITOR.dom.selection class has only getType() method and no type property. The type property is available in native selection object. While scrollIntoView is CKEDITOR.dom.selection method it means it is using non-existent property 🤔 You can check what is logged here - https://codepen.io/f1ames/pen/wvKZGBW?editors=1010.

We will have to take a closer look into this one.

@f1ames f1ames added core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. workload:medium and removed status:pending labels May 25, 2020
@f1ames f1ames changed the title Incorrect reference The scrollIntoView() method uses non-existent "selection.type" property May 25, 2020
@hub33k hub33k self-assigned this Jun 15, 2020
@jacekbogdanski
Copy link
Member

Closed in #4127

@jacekbogdanski jacekbogdanski added this to the 4.14.2 milestone Jul 2, 2020
@f1ames f1ames modified the milestones: 4.14.2, 4.15.0 Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants