Open Bug 92921 Opened 23 years ago Updated 8 months ago

contenteditable should not replace linebreaks by <br> inside a <pre>

Categories

(Core :: DOM: Editor, defect, P3)

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Milestone:
Future
Project Flags:
Webcompat Priority P3

People

(Reporter: glazou, Unassigned)

References

(Depends on 1 open bug, Blocks 4 open bugs)

Details

(Keywords: parity-chrome, parity-safari, topembed-, Whiteboard: [rules] EDITORBASE-) 

I can't find such a bug description in Bugzilla so...
In Composer, if you hit the CR key inside a <pre> element, it generates a <br>
element with no carriage return between the two lines of text around. It means
that such a pre :

     foo
     bar

is coded

     <pre>foo<br>bar</pre>

We should generate instead

     <pre>foo
     bar</pre>

because (a) linebreaks are meaningful inside a <pre>, (b) common practice is
not what we do today (c) we can end up with very long <pre> elements standing
on a single line
handing this one over to joe
Priority: -- → P3
Whiteboard: [rules]
Target Milestone: --- → mozilla1.0
jfrancis
Assignee: beppe → jfrancis
Ok, I have a fix for this bug. I worked on this bug because in the new AllTags
mode, we have the same problem for STYLE and SCRIPT elements.

Adding dependency to bug 88036 for this reason.
Blocks: 88036
-->Daniel
Assignee: jfrancis → glazman
Oops.  Don't land your fix unless it is clever indeed.  Users won't be able to 
click in blank lines inside pre unless we use <br>.  This bug should be made 
dependent on a (yet to be made) layout/caret/selection bug: getting caret working 
properly on blank lines caused by newlines.

perhaps 85505 counts as the bug this depends on.
Depends on: 85505
jfrancis: I remove the <br> only after first char's insertion. Following your
comment, I'll try to remove all chars on a line and put caret in this new
blank line. Will add comment here.
ROGNNNNTTTTUUDDDJJJUUUUUU !!!!! jfrancis : you were right, this causes problems.

This mandatory <br> on blank lines is really a blocking factor in a lot of
cases...
This bugs really deserves EDITORBASE in status whiteboard ; because of dependency
to bug 85505, I am unable to give a time estimate.
Whiteboard: [rules] → [rules] EDITORBASE
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
minusing, but would invite comments if anyone has any. mozilla shows the
resulting content correctly, as though there is no <br> tag was there (which
could be a bug in the layout engine?).
Whiteboard: [rules] EDITORBASE → [rules] EDITORBASE-
I strongly disagree with this minussing. Inserting BRs in PRE elements in one of
the very bad elements of comparison used by customers to say the HTML we produce
is lame.
In a PRE element, carriage returns *are* meaningful and we should not add BR
elements.

I also disagree with this minusing.  This affects users who copy/paste <pre>.

Possibly also related to this bug is the bug where we eat cr inside of
<textarea> tags.
Whiteboard: [rules] EDITORBASE- → [rules] EDITORBASE
Adding back the plus
Whiteboard: [rules] EDITORBASE → [rules] EDITORBASE+
Keywords: nsbeta1+
Keywords: topembed
Bulk moving all nsbeta1+ bugs which were targetted after Mozilla1.0 to Mozilla1.0
Target Milestone: mozilla1.0.1 → mozilla1.0
Marking topembed+
Keywords: topembed → topembed+
Why is this bug +'d?  We dont have a plan here, right?  This is totally dependant 
on layout changes that are not going to happen in the moz1.0 timeframe.  So 
what's the point of plussing it?

removing myself from the cc list
nsbeta1-, topembed-. This will require significant layout changes. Reassigning
to Marc for further investigation. Clearing + on EDITORBASE
Assignee: glazman → attinasi
Keywords: nsbeta1+, topembed+ → nsbeta1-, topembed-
Priority: P3 → P1
Whiteboard: [rules] EDITORBASE+ → [rules] EDITORBASE
Target Milestone: mozilla1.0 → Future
Job is too big for moz 1.0, minusing
Whiteboard: [rules] EDITORBASE → [rules] EDITORBASE-
I've noticed it too (Linux rc3 i386)
Steps to reproduce:
(0. Open "file>new>composer page")
1.Open "source"
2.Type:
<pre>




</pre>
3. Open "Preview"
4. Open "Source"

Expected results:
<pre>




</pre>

Actual results:
<pre><br><br><br><br><br></pre>

Completely wastes all efforts to make some decent ascii art ;)
bleah, sorry for spam (connection broken after first two lines of first comment
were displayed, thought it's all that was written on the subject)

If I could add my $0.02: What are the problems besides inability to click in the
editor on the <pre>'d area? Because I think keeping the same shape of a
preformatted document is much more important than ability to click on some area
(can you move into it using cursor keys?...) so I'd definitely prefer if you
fixed this, not looking at 85505's effects for now - small regression in one
area, significant gain in the other. Eventually file a new bug with the new
effect ("Area within <pre> tags is not clickable"). While keeping html
formatting of empty list items and table cells means little, with preformatted
text it's essential.

Eventually... Leave the conversion "as is" and when switching from other views
to editing html source and before saving document, convert the <br>s inside the
<pre> tag back to newlines. Not an ellegant method, but means a simple
workaround until 85505 is fixed.
I agree with Bartosz.
Daniel: do you still have the fix you talk about in comment #3? (no patch here!)
Returning to glazman for reconsideration.
Assignee: attinasi → glazman
Keywords: nsbeta1- → nsbeta1
This will never fly.  Most of our users don't even look at html source, but they 
sure notice when they can't click on a line.  We simply can't go forward here 
until layout issues are fixed.

It's much more important for mom and pop to be able to click in blank lines in 
plaintext mail reply than it is for html weenies to get the source they want (and 
I say that as an html weenie).

nsbeta1- based on comment 17
Keywords: nsbeta1 → nsbeta1-
Blocks: 174361
Compare bug 202444, on some behavior problems resulting from this.  I made it a
separate bug because this one concerns what we should do to the html source,
while 202444 is for making pre usable in composer regardless of how it's done.

Personally, after struggling with pre blocks for the past week, I suspect merely
losing the ability to set the caret on blank lines inside a pre would be a
welcome relief compared to what it does now.  (But ask me again after a week
with misbehaving caret. :-)

Hmm, how compartmentalized is the newline-to-br code?  Would it be hard to
disable it on a hidden pref, for people who use pre a lot and promise not to
file bugs about the caret, and then we could see how annoying it actually is? 
It's not like we don't have other caret placement problems that users live with.
This bug causes IE 6.0 and Mozilla 1.4 to render html differently.

When Composer replaces 2 consecutive CR's with <br><br> inside a pre tag,
for some reason IE only seems to care about the first <br>.
As a result, Mozilla renders the html page with the desired 2 CR's, IE with only 1.  

Chris
For embedded editors like Epoz (http://www.zope.org/Members/mjablonski/Epoz)
this bug is quite significant:
Many CMS rely on tidy to convert the embedded editors' output to xhtml. The
result for <pre> areas edited with mozilla is an added line between <pre> lines.

In Epoz' case this happens each time one switches from source design view to
source view as the source is run through tidy via xmlrpc when switching.

Gabriel
It's a problem for the WYSIWYG editing mode in the new version of Doctor, too:

http://doctor-test.mozilla.org/doctor.cgi?action=edit&file=docs/tutorials/tinderstatus

(Load that URL, then click "Show Diff" to see the damage.)
I find this bug extremely irritating both because the HTML source view becomes
extremely wide while editing the <pre> section, and because that's not the only
editor I use on the HTML source.  For instance, sometimes I will want to make
small adjustments using vi; but it's limited to a max line length of 4096 chars
or so, which can easily be exceeded by a large <pre> section with only <br>'s to
separate the lines.  That can make the file completely uneditable (and no, emacs
is not the proper solution; the mangled text shape is still disturbing).
This bug has been around for almost 5 years!  With the increasing adoption of WYSIWYG web-based editors like FCKEditor (used in many Wikis now), this bug presents a significant usability hurdle.

Our organization is using MediaWiki w/FCKEditor extension, and I really have no other choice at this point than to steer our users to Internet Explorer for interacting with the Wiki, even though many of them have converted over to Firefox.

The result of this bug is that FCKEditor simply produces incorrect pages in the Wiki.  If somebody creates a 'formatted' block (read <PRE>), the end result is that something like this is displayed in the Wiki:

   This is a preformatted<br/>block of text<br/>with linebreaks<br/>

It "looks" correct in the editor, but the saved page renders as above.

(In reply to comment #31)

>    This is a preformatted<br/>block of text<br/>with linebreaks<br/>

This markup _is_ valid. And if MediaWiki w/FCKEditor is unable to deal with it,
that's a bug in MediaWiki w/FCKEditor.
> This markup _is_ valid. And if MediaWiki w/FCKEditor is unable to deal with 
> it, that's a bug in MediaWiki w/FCKEditor.

It's true that the markup is valid, and perhaps FCKEditor should deal with it, but it still seems like it would be better if it (and other clients which use editor functionality) didn't have to deal with it, especially when the common expectation is for line breaks in PRE blocks to be represented by CR characters.
Yeah, saying the markup is valid is a straw man.  No one said otherwise (valid in a well-formed sense).  But the <br> insertion madness is a huge bug, never mind well-formedness!  Daniel, how hard is it to fix?

/be
Brendan: this bug isn't hard (Daniel had a fix years ago -- see comment 3), but bug 85505, on which this depends, requires someone who can spend the time to dig into the gecko frame and caret code. That would solve not only this bug but lots of other longstanding editor annoyances as well.
(In reply to comment #35)
> Brendan: this bug isn't hard (Daniel had a fix years ago -- see comment 3), but
> bug 85505, on which this depends, requires someone who can spend the time to
> dig into the gecko frame and caret code. That would solve not only this bug but
> lots of other longstanding editor annoyances as well.

Caret and frame code are being fixed (finally).  Roc and mrbkap are on it, it is happening for 1.9.  Plan this bug's fix accordingly.

/be
We can also do something in the serializer...
(In reply to comment #35)
> Brendan: this bug isn't hard (Daniel had a fix years ago -- see comment 3), but
> bug 85505, on which this depends, requires someone who can spend the time to
> dig into the gecko frame and caret code. That would solve not only this bug but
> lots of other longstanding editor annoyances as well.
> 

My patch for bug 314519 makes placing the caret inside empty lines in preformatted text possible (using arrow keys. Using the mouse works already, as far as I can tell).
So, if that patch is accepted, I don't think we need to wait for a full fix to bug 85505 (whatever that actually means) in order to fix this one.
Depends on: 314519
(In reply to comment #38)
> Using the mouse works already, as far as I can tell.

For the record, what fixed the mouse-click case was the fix for bug 298690 (on 2005-10-31).
(In reply to comment #3)
> Ok, I have a fix for this bug. I worked on this bug because in the new AllTags
> mode, we have the same problem for STYLE and SCRIPT elements.

Daniel, do you still have that fix? If so, I think it could be used, now that the relevant parts of bug 85505 are fixed.
QA Contact: sujay → editor
Daniel, do you mind if I assign it to myself?
(In reply to comment #41)
> Daniel, do you mind if I assign it to myself?

Of course not! Done.
Assignee: daniel → ehsan
old bug.  down to P3
Priority: P1 → P3
Assignee: ehsan → nobody
Summary: composer should not replace CRs by <br> inside a <pre> → contenteditable should not replace linebreaks by <br> inside a <pre>

Note that Chrome inserts <div> instead of <br>.

(In reply to Kagami :saschanaz from comment #45)

Note that Chrome inserts <div> instead of <br>.

Odd. I tested this with Chrome 91, chrome inserts another <pre> for insertParagraph (Enter key press).

And insert \n for insertLineBreak instead of <br> (Shift + Enter key press).

Perhaps, for comm-central products, we should add an API to keep current behavior for insertParagraph, but we should follow the Chrome's behavior for web-compat by default.

Blocks: editor-blink-compat
Severity: normal → S3

Safari behaves similarly to Chrome. So, we should align the behavior.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Keywords: parity-safari
Webcompat Priority: --- → ?
Webcompat Priority: ? → P1

The severity field for this bug is set to S3. However, this bug has a P1 WebCompat priority.
:masayuki, could you consider increasing the severity of this web compatibility bug?

For more information, please visit auto_nag documentation.

Flags: needinfo?(masayuki)

If this breaks some web apps in the wild, yes, we should make this at least P2 and I'd work on this immediately.

twisniewski: Do you know such web apps?

Assignee: masayuki → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(masayuki) → needinfo?(twisniewski)

I don't know of specific modern web apps off the top of my head, no. I think the P1 priority can be downgraded if there are more important contenteditable bugs to work on first.

Flags: needinfo?(twisniewski)

Asking for downgrading webcompat-priority per comment 49 and comment 50

Webcompat Priority: P1 → ?

The severity field for this bug is relatively low, S3. However, the bug has 12 votes.
:hsinyi, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(htsai)
Webcompat Priority: ? → P3
Flags: needinfo?(htsai)

I have run into this issue as well and it was a source of a difficult to detect bug. As said earlier, both Safari and Chromium preserve newlines and Firefox should do the same. The current behavior creates an inconsistency in every website that uses a contenteditable-based text editor, which is common (in my case it was TinyMCE 5), when inserting preformatted text as the resulting content from the editor will be different between browsers, leading to unexpected scenarios. Due to this, I think the issue priority should be increased.

You need to log in before you can comment on or make changes to this bug.