Opened 12 years ago
Closed 12 years ago
#9981 closed New Feature (fixed)
Implement fragment#filter
Reported by: | Piotrek Koszuliński | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.1 RC |
Component: | Core : Parser | Version: | |
Keywords: | Cc: |
Change History (6)
comment:1 Changed 12 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | new → assigned |
comment:2 Changed 12 years ago by
Status: | assigned → review |
---|
comment:3 Changed 12 years ago by
Status: | review → review_failed |
---|
There is a change on a previously existing TC which looks wrong. It's inside dt/core/htmlparser/filter.html, in 'test elementNames rules' at line 150,
The element name rule [ 'foo', 'bar' ] must have a full match. This means that <foo> becomes <bar>, but <foofoo> must stay intact. Actually both the old and the new tests seem to be wrong (both <barfoo> and <barbar>).
If a partial match would be ever required, a regex should be used, not a string.
comment:4 Changed 12 years ago by
Status: | review_failed → review |
---|
Unfortunately this was broken from the very beginning. My changes in this branch just fixed the filter so it applies filter as long as name is changed - exactly like for elementNames
.
When writing tests for #9972 I found also this weird behaviour that string is used as a replace() pattern, so it doesn't mean full match. Initially I also found this completely wrong. But then I checked if there's a chance to fix it without affecting performance and I think that it'd better to leave this simple mechanism. First element in array is used as a replace pattern (replace() accepts string and regexp), second element as replacement. That's all.
What's more - our code isn't using string pattern (only one usage in htmlDataProcessor, but rather safe). Instead, it uses regexp correctly.
So, I'm to leave this as expected behaviour. This is how it worked, this is fast and the only wrong thing is that it wasn't documented so it may be a little confusing.
comment:5 Changed 12 years ago by
Status: | review → review_passed |
---|
comment:6 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Commited into major with git:6c8624e.
Pushed t/9981 on dev and tests.