Opened 17 years ago
Closed 16 years ago
#1229 closed Bug (fixed)
<pre> should be merged when applied to multiple paragraphs
Reported by: | Vialis | Owned by: | Martin Kou |
---|---|---|---|
Priority: | Normal | Milestone: | FCKeditor 2.6.3 |
Component: | Core : Styles | Version: | FCKeditor 2.4.3 |
Keywords: | Confirmed Review+ | Cc: |
Description
When I use the format "Formatted" every newline leads to a new "Formatted" text block. I would expect that all text that is entered will be shown in the samen text block.
Attachments (4)
Change History (22)
comment:1 Changed 17 years ago by
Component: | General → Core : Styles |
---|---|
Keywords: | Discussion added |
Milestone: | → FCKeditor 2.6 |
Summary: | FCKeditor and Mediawiki: when using format "Formatted" every newline leads to a new "Formatted" text block → <pre> should be merged when applied to multiple paragraphs |
comment:2 Changed 17 years ago by
How about making the <pre> -> <p> logic like the conversion login in the "Paste As Text" dialog:
- If two lines are separated by one line break, change the line break to <br>.
- If two lines are separated by two line breaks, change the line break to a new block.
Note that some <pre> -> <p> conversion logic has been written in the patch to #1355, it already does the <br> conversion, but it doesn't do the new block conversion yet.
comment:3 Changed 17 years ago by
Your idea sounds good Martin.
Btw, my previous example was wrong. The following is correct instead:
Initial structure: <p>Line 1</p> <p>Line 2</p> After Format change: <pre>Line 1 Line 2</pre>
comment:4 Changed 17 years ago by
Keywords: | Confirmed added; Discussion removed |
---|
comment:5 Changed 17 years ago by
Owner: | set to Martin Kou |
---|---|
Status: | new → assigned |
Changed 17 years ago by
Attachment: | 1229.patch added |
---|
comment:6 Changed 17 years ago by
Keywords: | Review? added |
---|
comment:7 Changed 17 years ago by
Keywords: | Review? removed |
---|
Review request retracted because it was found that the patch can be simplified after a discussion with Fred.
comment:8 Changed 17 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed with [1790].
Click here for more info about our SVN system.
comment:9 Changed 17 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Changed 17 years ago by
Attachment: | 1229_2.patch added |
---|
comment:11 Changed 17 years ago by
Keywords: | Review? added |
---|
comment:12 Changed 17 years ago by
Keywords: | Review- added; Review? removed |
---|
The merging is broken on IE (requires the outerHTML trick to set the innerHTML). There is also part of the logic that I couldn't easily understand, and therefore would require some discussion.
Changed 17 years ago by
Attachment: | 1229_3.patch added |
---|
comment:13 Changed 17 years ago by
Keywords: | Review? added; Review- removed |
---|---|
Milestone: | FCKeditor 2.6 → FCKeditor 2.6.1 |
I've attached a new patch for it, derived from Martin's patch. I should discuss about it with Martin at this point to be sure things are ok, possibly merging parts of his patch to it to come with a final solution.
We'll need to do that calmly, so I'm postponing it a bit to not block the release of the 2.6.
comment:14 follow-up: 15 Changed 17 years ago by
Two flaws in the patch found:
- The way _CheckAndMergePre() calculates its own previous block instead of getting the previous block from the FCKDomRangeIterator loop makes it possible for non-selected <pre> blocks to get merged.
- While the logic of the _CheckAndSplitPre() in 1229_3.patch looks identical to the one in 1229_2.patch, it has a very subtle bug - cursor advancement is incorrectly handled when lastNewBlock exists. The bug causes _CheckAndSplitPre() to be unable to split a converted block to more than 2 parts, and also causes it to rearrange the order of the blocks, which is very bad.
I've included a fixed patch for review.
Changed 17 years ago by
Attachment: | 1229_4.patch added |
---|
comment:15 Changed 17 years ago by
Keywords: | Review+ added; Review? removed |
---|
Replying to martinkou:
The way _CheckAndMergePre() calculates its own previous block instead of getting the previous block from the FCKDomRangeIterator loop makes it possible for non-selected <pre> blocks to get merged.
That was actually the intention of it. But anyway, this is subject to interpretation. So, let's put it in production and catch the feedback. This is a strong benefit anyway.
comment:16 Changed 17 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Fixed with [2220].
Click here for more info about our SVN system.
comment:17 Changed 16 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Hi
It seems that there is again a bug with <pre> tag in mediawiki Test on : http://mediawiki.fckeditor.net/index.php/Sandbox enter the following text then switch several times between wikitext
<pre> Hello Wiki TS User Hello Wiki TS User Hello Wiki TS User </pre> <pre> Bonjour Utilisateur Wiki TS Bonjour Utilisateur Wiki TS Bonjour Utilisateur Wiki TS Bonjour Utilisateur Wiki TS </pre>
comment:18 Changed 16 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
@CKL,
This is an old ticket that has already been fixed and released. Please open a new ticket for it.
We could prospect the following:
The problem here is, what should happen when switching from <pre> back to <p>? I would expect it to generate a single <p> with <br> for each line break, just like this:
But them people will complain that the transformation to <pre> and back to <p> destroys the initial structure. But it sounds like the way to go in any case. Any thoughts on this?