-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
table elements (td, tr, th, ..) with an id that starts with dot (.) causes JavaScript runtime err #681
Comments
@cschettle I can't reproduce this issue with CKEditor version 4.7.0 and 4.7.1. Could you please provide codepen, jsbin or jsfiddle example with configuration and exact steps to reproduce? |
I updated the original issue to be more descriptive and provide additional information. I have attached a smaller .htm file (table_small.txt) for your testing... (the original test.htm file has duplicate Id's which might confuse the issue) |
@cschettle Thanks for your response. Could you please take a look at this demo https://jsfiddle.net/o166vxma/? Please run this on IE11 and follow your steps. Selecting cells doesn't cause an error in the developer console. |
Hi,
I have run the fiddle demo using IE and it DOES reproduce the error. After starting the demo with IE, open the developer tools (F12). You must select the cell (highlight the number so there is a selection in the dom) to cause the error. I have attached a snapshot of the error and how to select the cell.
This is a very subtle problem and will not give the expected results (always returns null) even if you have selected nested table elements which is what the code is checking for. As I mentioned previously, you can verify that the css selector that is generated when there is a dot in the cell id is not valid by verifying the selector with a third party test tool.
Thanks for taking the time to put the demo on fiddle.
Regards,
Chris Schettle
From: beatadelura [mailto:notifications@github.com]
Sent: Tuesday, August 01, 2017 12:43 AM
To: ckeditor/ckeditor-dev <ckeditor-dev@noreply.github.com>
Cc: cschettle <chris_schettle@hotmail.com>; Mention <mention@noreply.github.com>
Subject: Re: [ckeditor/ckeditor-dev] table elements (td, tr, th, ..) with an id that starts with dot (.) causes JavaScript runtime err (#681)
@cschettle</~https://github.com/cschettle> Thanks for your response. Could you please take a look at this demo https://jsfiddle.net/o166vxma/? Please run this on IE11 and follow your steps. Selecting cells doesn't cause an error in the developer console.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#681 (comment)>, or mute the thread</~https://github.com/notifications/unsubscribe-auth/ANCHnQaUSMpOIC6n1uazJ2r9h5jg4wPeks5sTtb6gaJpZM4Okkxs>.
|
@cschettle Thank you for adding more information. I see the problem now and I can reproduce this issue. |
Glad that you can see the issue. Thank you.
From: beatadelura [mailto:notifications@github.com]
Sent: Monday, August 07, 2017 12:51 AM
To: ckeditor/ckeditor-dev <ckeditor-dev@noreply.github.com>
Cc: cschettle <chris_schettle@hotmail.com>; Mention <mention@noreply.github.com>
Subject: Re: [ckeditor/ckeditor-dev] table elements (td, tr, th, ..) with an id that starts with dot (.) causes JavaScript runtime err (#681)
@cschettle</~https://github.com/cschettle> Thank you for adding more information. I see the problem now and I can reproduce this issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#681 (comment)>, or mute the thread</~https://github.com/notifications/unsubscribe-auth/ANCHncrF3HOEx-AXQp3jxJtAb2VITDtyks5sVsHLgaJpZM4Okkxs>.
|
I see this is not yet fixed in the ckeditor 4.7.3 Here is how I fixed the code in ckeditor\dom\element.js |
Hi @cschettle, |
The problem is in the implementation of Note that #641 could be caused by the same issue: incomplete implementation of |
Hi Team. It seems the bug has not been fixed. Based on the conversation above, seems someone has been already looking into this. Do we have any updates? Thanks a lot! |
Closed with: #4666 |
Are you reporting a feature or a bug?
Bug
Check if the issue is already reported
Put all reference links here…
Provide detailed reproduction steps (if any)
Expected result
What is the expected result of the above steps?
The table cell is selected (highlighted)
Actual result
What is the actual result of the above steps?
Only IE 11 throws this exception :
0x800a139e - JavaScript runtime error: SyntaxError
in ckeditor.js (minified version) or in dom\element.js in the source version
in findOne (or also find) at line 4024
Other details
The syntax error is a result of a bad CSS selector being returned from
getContextualizedSelector( this, selector ) (see element.js) While FF and Chrome do not throw an exception, the selector does would not work properly. The selector produced using the attached table.htm file (and selecting any cell with an id=.amt.3 is
#.amt.3 td, #.amt.3 th, #.amt.3 tr, #.amt.3 tbody, #.amt.3 table
You can test this selector using an online tool like https://try.jsoup.org/
I have attached the following files:
table.txt html file that contains table with cells that have id's with dot
IE11_syntax_err.PNG shows err in IE11 dev tools with ckeditor minified version
IE11_syntax_err_src.PNG shows err in IE11 dev tools with ckeditor source
bad_selector.PNG shows value of selector returned from getContextualizedSelector which is called from find and findOne in dom\element.js
table.txt
The original code:
return '#' + id + ' ' + selector.split(/,\s*/).join(', #' + id + ' ');
The #.amt for example is not a valid selector (would need to be escaped)
I modified the code to use the attribute style selector instead of id # as follows:
return '[id="' + id + '"] ' + selector.split(/,\s*/).join(', [id="' + id + '"] ');
The text was updated successfully, but these errors were encountered: