Opened 14 years ago
Closed 14 years ago
#7513 closed Bug (fixed)
Invalid markup in STYLE make CK crash
| Reported by: | Dagfinn Bergsager | Owned by: | Frederico Caldeira Knabben |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.5.4 |
| Component: | Core : Parser | Version: | 3.1 |
| Keywords: | Cc: |
Description
If i click the SOURCE-botton and insert <p style="color: red;>Red</p> (missing the last ") and then click SOURCE again to go back to normal edit-view; CK will go grey and stop working.
Attachments (2)
Change History (14)
comment:1 Changed 14 years ago by
| Status: | new → pending |
|---|
comment:2 Changed 14 years ago by
I can reproduce it on http://nightly.ckeditor.com/6640/_samples/fullpage.html
And I pasted <p style="color: red;>Red</p> after <body>
Now I see that this only bug in Firefox4 and Chrome10 (tested on Win7 and Mac). IE and Opera change/remove the markup.
This might though be a bug in chrome and firefox4's edit components...
comment:3 Changed 14 years ago by
| Status: | pending → confirmed |
|---|---|
| Version: | → 3.1 |
If you paste the following code:
<p style="color: red;>Red</p>
before the sample text (E.g. in replaceByCode or fullPage samples) so that it looks like this:
<p style="color: red;>Red</p> <p>This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
Than Firefox4 and Chrome 10.0.648.204 will hang.
The error from fireBug is: "regular expression too complex while ( ( parts = this._.htmlPartsRegex.exec( html ) ) ) " htmlparser.js line:127
comment:4 Changed 14 years ago by
| Milestone: | → CKEditor 3.5.4 |
|---|---|
| Priority: | Normal → High |
comment:5 Changed 14 years ago by
On other browsers:
<p ckeditor.com="" href="http://ckeditor.com/" http:="" style="color: red;"> CKEditor.</p>
Still better than a crash.
Changed 14 years ago by
| Attachment: | 7513.patch added |
|---|
comment:6 Changed 14 years ago by
| Component: | General → Core : Parser |
|---|---|
| Owner: | set to Garry Yao |
| Priority: | High → Normal |
| Status: | confirmed → review |
Propose for simplified html parser tokenized, reduce the complexity of attributes matcher (which consist of nested wildcards which are considered harmful for regexp engine), as the attributes parsing will be later handled by "attribsRegex".
comment:7 Changed 14 years ago by
| Status: | review → review_failed |
|---|
I was working to figure out why we have the attributes matching on the regex (there must be a reason). In fact, it is there to handle inputs like this:
<p title="a < b > c" class="test">Sample</p>
After applying the patch, the above HTML will not be handled properly on IE and Opera.
I have introduced a test for this case with [6742]:
http://ckeditor.t/dt/core/htmlparser/htmlparser.html
I've found a minor simplification to the regex that seems to work for this ticket. I'll ask for review on it.
Changed 14 years ago by
| Attachment: | 7513_2.patch added |
|---|
comment:8 Changed 14 years ago by
| Owner: | changed from Garry Yao to Frederico Caldeira Knabben |
|---|---|
| Status: | review_failed → review |
comment:9 follow-up: 10 Changed 14 years ago by
| Status: | review → review_failed |
|---|
While fixing your inputted TC, the patch will not parse the following source, degrade it as a single text node:
<span style=""">text</span>
comment:10 Changed 14 years ago by
| Status: | review_failed → review |
|---|
Replying to garry.yao:
While fixing your inputted TC, the patch will not parse the following source, degrade it as a single text node:
<span style=""">text</span>
This also happens with the current code. It's another edge case, which is not related to this ticket. At least no data loss happens here.
If you feel it's worth, please have a ticket for it. I would not waste my time on that though, without having some outsider confirming this as a valid issue.
comment:11 Changed 14 years ago by
| Status: | review → review_passed |
|---|
It's just far more frequently occurred than your inputted TC, anyway R+ current, #7693 filed.
comment:12 Changed 14 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [6761].
Could you tell me which version of CKEditor are you using and in what browser does the issue occur?
I'm asking because I have checked all browsers and every time I've pasted your code, it was removed (was not parsed) and browser did not hang. CKEditor version I have used is the latest stable - 3.5.2.
Can you still reproduce the issue in latest demo version ? http://nightly.ckeditor.com/demo Perhaps this problem has already been fixed? If it has been fix than my advise is that you should upgrade to newer version. If not please provide me with more detailed information on how to reproduce this bug.