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

Status: NEW

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

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 1

[rubydoo123]

handing this one over to joe

Comment 2

[rubydoo123]

jfrancis

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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.

Comment 4

[Kathleen :Brade]

-->Daniel

Comment 5

[Joe Francis]

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.

Comment 6

[Joe Francis]

perhaps 85505 counts as the bug this depends on.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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.

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

ROGNNNNTTTTUUDDDJJJUUUUUU !!!!! jfrancis : you were right, this causes problems.

This mandatory <br> on blank lines is really a blocking factor in a lot of
cases...

Comment 9

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

This bugs really deserves EDITORBASE in status whiteboard ; because of dependency
to bug 85505, I am unable to give a time estimate.

Comment 10

[Asa Dotzler [:asa]]

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)

Comment 11

[Syd Logan]

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?).

Comment 12

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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.

Comment 13

[Kathleen :Brade]

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.

Comment 14

[Syd Logan]

Adding back the plus

Comment 15

[Kevin McCluskey (gone)]

Bulk moving all nsbeta1+ bugs which were targetted after Mozilla1.0 to Mozilla1.0

Comment 16

[Kevin McCluskey (gone)]

Marking topembed+

Comment 17

[Joe Francis]

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?

Comment 18

[rubydoo123]

removing myself from the cc list

Comment 19

[Kevin McCluskey (gone)]

nsbeta1-, topembed-. This will require significant layout changes. Reassigning
to Marc for further investigation. Clearing + on EDITORBASE

Comment 20

[Syd Logan]

Job is too big for moz 1.0, minusing

Comment 21

[Bartosz Wucke]

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 ;)

Comment 22

[Bartosz Wucke]

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.

Comment 23

[Charles Manske]

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.

Comment 24

[Joe Francis]

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).

Comment 25

[Kevin McCluskey (gone)]

nsbeta1- based on comment 17

Comment 26

[Akkana Peck]

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.

Comment 27

[Christophe Cornu]

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

Comment 28

[Gabriel Wicke]

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

Comment 29

[Myk Melez [:myk] [@mykmelez]]

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.)

Comment 30

[eponymousalias]

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).

Comment 31

[Brett Wooldridge]

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.

Comment 32

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(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.

Comment 33

[Myk Melez [:myk] [@mykmelez]]

> 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.

Comment 34

[Brendan Eich [:brendan]]

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

Comment 35

[Akkana Peck]

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.

Comment 36

[Brendan Eich [:brendan]]

(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

Comment 37

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

We can also do something in the serializer...

Comment 38

[Uri Bernstein]

(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.

Comment 39

[Uri Bernstein]

(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).

Comment 40

[Uri Bernstein]

(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.

Comment 41

[(no longer active)]

Daniel, do you mind if I assign it to myself?

Comment 42

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #41)
> Daniel, do you mind if I assign it to myself?

Of course not! Done.

Comment 44

[Makoto Kato [:m_kato]]

old bug.  down to P3

Comment 45

[Kagami Rosylight [:saschanaz] (they/them)]

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

Comment 46

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(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.

Comment 47

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

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

Comment 48

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 49

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

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?

Comment 50

[Thomas Wisniewski [: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.

Comment 51

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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

Comment 52

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 53

[mohd.akram]

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.