Opened 10 years ago
Closed 10 years ago
#11533 closed Bug (fixed)
Elements upcasted repeatedly if pseudo-DOM modified during upcast
| Reported by: | Olek Nowodziński | Owned by: | Olek Nowodziński |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.3.3 |
| Component: | General | Version: | 4.3 |
| Keywords: | Cc: |
Description
It turned out that in some cases upcasting may go out of control.
Assumptions:
- Let's start with the following html:
<body><p>x<img/></p></body>
- Each <img> is to be upcasted to <figure><img></figure>.
Now let's follow forEach, which calls the iterator (see: createUpcastIterator) during the upcast process, step-by-step:
- ForEach: Visit <p>.
- ForEach: Visit children of <p>.
- ForEach: "x" text found
- Upcast: Ignore text.
- ForEach: Visit <img>
- Upcast: Start upcasting <img> to <figure>.
- Upcast: Split <p> because <figure> is block.
<body><p>x</p><p><img/></p></body>
- Upcast: Replace the parent of <img> (<p>) with <figure>
<body><p>x</p><figure><img/></figure></body>
- Widget system: Remember that <figure> is to be wrapped. Great. But problems start now.
- ForEach: No more children of <p>.
- ForEach: Go back to <body>.
- ForEach: Move to <figure>, which is the next child of <body>
- ForEach: Enter <figure> (it's not a widget yet).
- ForEach: Visit children of <figure>.
- ForEach: Visit <img>.
- Upcast: Start upcasting <img>.
- Bang! Upcasting the same <img> once again, while it belongs to <figure> which is to be a widget.
Change History (8)
comment:1 Changed 10 years ago by
| Owner: | set to Olek Nowodziński |
|---|---|
| Status: | new → assigned |
comment:2 Changed 10 years ago by
comment:4 Changed 10 years ago by
| Status: | assigned → review |
|---|
comment:6 Changed 10 years ago by
| Status: | review_failed → review |
|---|
- I updated other Widget/Image2 test to the new
data-cke-widget-upcastedattribute API. - I improved the fix to avoid infinite loop when
<img>is replaced by<figure><img></figure>during upcast but additionally<figure>can be upcasted.
comment:7 Changed 10 years ago by
| Status: | review → review_passed |
|---|
Code looks good, tests pass, widgets work correctly and the patch is clear.
I was thinking about removing data-cke-widget-upcasted in htmlFilter (it's applied just after upcasting), so we would avoid any theoretically possible problems with handling widget which already was upcasted (e.g. pasting a widget), but at the same time this would affect performance, so let's wait and see if any real issues will appear.
comment:8 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
git:fff2039 landed in master (17549d9 tests).
Note: See
TracTickets for help on using
tickets.
cc