Opened 10 years ago
Closed 10 years ago
#12141 closed Bug (fixed)
List is lost when indenting in <li><p>
Reported by: | Piotrek Koszuliński | Owned by: | Artur Delura |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.4.6 |
Component: | General | Version: | 4.2 |
Keywords: | Cc: |
Description (last modified by )
- Set source:
<ul> <li><p>First</p></li> <li><p>Second</p></li> </ul>
- Place caret in the first item.
- Press indent button.
Second item is lost.
- Pull request: https://github.com/ckeditor/ckeditor-dev/pull/108
- An analysis how ident* plugins should handle this case: https://github.com/ckeditor/ckeditor-dev/pull/108#issuecomment-47332860
Change History (13)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 10 years ago by
Version: | → 4.2 |
---|
comment:4 Changed 10 years ago by
Milestone: | → CKEditor 4.4.6 |
---|
comment:5 Changed 10 years ago by
Owner: | set to Artur Delura |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 10 years ago by
Status: | assigned → review |
---|
Pull request from lykahb was almost fine. His commit fixed issue described in this test (to write this one I had to make firstItemInPath
public). The remaining problem was following: When indentblock
plugin was enabled, caret was at the first list item and list item include paragraph, then indent button was disabled. I fixed this issue and unified behaviour with one when list item didn't include paragraph. So now when user click indent button, then whole list is indented. Fix is here - instead of disabling button I switch context to closest li
. Changes in branch:t/12141.
comment:7 follow-up: 8 Changed 10 years ago by
Status: | review → review_failed |
---|
- Docs are written incorrectly. First of all, make sure to generate it, because your additions throw errors. Second, read about the order of
@tags
- http://docs.ckeditor.com/#!/guide/dev_code_documentation. Third, document what thequery
argument is (you can simply link to the elementPath.contains doc). - The name of the new test file is incorrect.
- Add more tests for the changes in the indent* plugins. You should check what indentblock does, what indentlist does, what both do together. And also you should check cases like selection in second paragraph in list item, in the second list item, etc - so cases which should not be touched by the change you did.
comment:8 Changed 10 years ago by
Replying to Reinmar:
- Docs are written incorrectly. First of all, make sure to generate it, because your additions throw errors. Second, read about the order of
@tags
- http://docs.ckeditor.com/#!/guide/dev_code_documentation. Third, document what thequery
argument is (you can simply link to the elementPath.contains doc).
Done
- The name of the new test file is incorrect.
Done
- Add more tests for the changes in the indent* plugins. You should check what indentblock does, what indentlist does, what both do together. And also you should check cases like selection in second paragraph in list item, in the second list item, etc - so cases which should not be touched by the change you did.
I have written 8 tests (2 * 2 * 2) for various cases:
- whether
indentBlock
is enbled. - whether list items are wrapped in
p
element. - whether caret is at the first item.
Chages and tests in branch:t/12141.
comment:9 Changed 10 years ago by
Status: | review_failed → review |
---|
comment:10 Changed 10 years ago by
Summary: | [PR#108] List is lost when indenting in <li><p> → List is lost when indenting in <li><p> |
---|
comment:11 Changed 10 years ago by
I rebased the branch:t/12141 and added one more commit there (cleanup - make sure to check what I changed).
On the last meetup we talked about adding manual tests with basic samples which will let us to quickly check the TC without the need to build your own samples. This ticket requires testing 3 configurations and only one is available in our samples.
Could you create 3 manual tests (in Bender) with short description what should be checked?
comment:13 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Good work! Fixed on master with git:a112253.
#12467 was marked as duplicate.
Same problem can be reproduced with below code:
Problem can be reproduced from CKEditor 4.2.