Opened 16 years ago
Closed 16 years ago
#3730 closed Bug (fixed)
Indent and BR selections
| Reported by: | Damian | Owned by: | Garry Yao |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.0 |
| Component: | General | Version: | |
| Keywords: | IBM Confirmed Review+ | Cc: |
Description
When selecting text that uses <BR> for line separation, the indent feature bleeds the selection to the containing <P>. So that this:
<div> test 1<br /> test 2<br /> test 3<br /> test 4<br /> test 5<br /> </div>
becomes this;
<div style="margin-left: 40px;"> test 1<br /> test 2<br /> test 3<br /> test 4<br /> test 5<br /> </div>
irrespective of how many lines of text were selected. The expected behavior would be to indent only the selected text. E.g. if line 3 & 4 were selected the result could be:
<div> test 1<br /> test 2<br /> <div style="margin-left: 40px;"> test 3<br /> test 4<br /> </div> test 5<br /> </div>
Attachments (3)
Change History (15)
comment:1 Changed 16 years ago by
| Keywords: | Discussion added |
|---|
comment:2 Changed 16 years ago by
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
This actually does not make sense when we look at the HTML specification - paragraphs are not supposed to contain other paragraphs. Let's say we feed this XHTML code to W3C's XHTML validator.
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-type" content="text/html;charset=UTF-8" />
<title>Test XHTML validity</title>
</head>
<body>
<p>
test 1<br />
test 2<br />
<p style="margin-left: 40px;">
test 3<br />
test 4<br />
</p>
test 5<br />
</p>
</body>
</html>
W3C will tell you that this isn't valid XHTML. And in fact, even if you force that HTML into the browser via Firebug or a raw HTML file on your local harddisk - the paragraph will look wrong.
Changing the inner block to a <div> would make it look "right" but it's still invalid XHTML.
comment:3 follow-up: 4 Changed 16 years ago by
| Resolution: | invalid |
|---|---|
| Status: | closed → reopened |
I agree that nested <P> tags would cause invalid XHTML. On the other hand, if my document does not contain <P> tags but <DIV> tags and I used enterMode=BR, then I would expect that indent would work on my selected lines of text. The example provided seems like a valid use case to me. Am I missing something?
Reopened to continue discussion.
comment:4 Changed 16 years ago by
Replying to damo:
I agree that nested <P> tags would cause invalid XHTML. On the other hand, if my document does not contain <P> tags but <DIV> tags and I used enterMode=BR, then I would expect that indent would work on my selected lines of text. The example provided seems like a valid use case to me. Am I missing something?
Reopened to continue discussion.
Just to clarify, the example I am referring to is the code snippet with <DIV> tags.
comment:5 Changed 16 years ago by
| Keywords: | Confirmed added; Discussion removed |
|---|
The missing information in the report is that this issue happens with enterMode=br. In this case, we should really have it working as the reporter described.
comment:6 Changed 16 years ago by
| Owner: | set to Martin Kou |
|---|---|
| Status: | reopened → new |
Changed 16 years ago by
| Attachment: | 3730.patch added |
|---|
comment:7 follow-up: 9 Changed 16 years ago by
| Keywords: | HasPatch added |
|---|
After discuss with Martin, we could provide a very simply way to go:
- If content is directly inside body(blockLimit), indent the selected lines separated by <br> as a whole, with a new <div> block;
- If content is otherwise inside a block, indent the existed block.
comment:8 Changed 16 years ago by
| Keywords: | HasPatch removed |
|---|---|
| Owner: | changed from Martin Kou to Garry Yao |
| Status: | new → assigned |
After a group discussion, we need some changes for the second case, it should work as same with the ticket described, I'm taking over the ticket.
Changed 16 years ago by
| Attachment: | 3730_2.patch added |
|---|
comment:9 Changed 16 years ago by
| Keywords: | Review? added |
|---|
Replying to garry.yao:
After discuss with Martin, we could provide a very simply way to go:
- If content is directly inside body(blockLimit), indent the selected lines separated by <br> as a whole, with a new <div> block;
Remain unchanged.
- If content is otherwise inside a block, indent the existed block.
Change to : If indent the selected lines group as 1. and break the block.
Changed 16 years ago by
| Attachment: | 3730_3.patch added |
|---|
comment:11 Changed 16 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
comment:12 Changed 16 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
This's been designed by purpose, but maybe your expected result will a better result for enterMode=BR.