1058446 - ContentEventHandler should append a line break at the end of <p> element

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Currently, ContentEventHandler appends line breaks at <br> found.

However, in our HTML editor, "foo" + Enter + "bar" causes following HTML content:

<p>foo</p>
<p>bar<br></p>

Current ContentEventHandler generates flat text from this as "foobar\n". This is too bad because setting focus before "bar" and after "foo" is equivalent. Actually, selection set event can move caret only after "foo".

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Um, it might be better to check primary frame's display. when it's not inline, inserting a line break at start of the content and end of the content allows to specify any point from flat text offset. The demerit of this approach is that the generated text becomes longer in rich text editors.

For example,

<p>A<img>B</p>
<div>C<p>D</p>E</div>

is currently represented as "ABCDE".

If we insert line breaks at start of each element, then, it becomes: "\nA\nB\nC\nDE".

If we insert line breaks at end of each element, then, it becomes: "A\nB\nCD\nE\n".

If we insert line breaks at both start and end, then, it becomes: "\nA\nB\n\nC\nD\nE\n".

However, looks like that "</p><div>" shouldn't cause inserting \n twice because in our HTML editor, users cannot move caret between them. It may cause bug if IME sets selection between them.

So, I think that if previous sibling of a block is block, it shouldn't cause "\n\n".

Inserting start of non-inline box is easy. However, inserting line break to end of boxes is difficult. I have no idea how to do it with content iterator.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

It seems fixing this could potentially also fix bug 1145224.

> it might be better to check primary frame's display.

Probably easier to just check frame->IsFrameOfType(nsIFrame::eLineParticipant)

> Inserting start of non-inline box is easy. However, inserting line break to
> end of boxes is difficult. I have no idea how to do it with content iterator.

I'm not sure why inserting at the start is easier. We have two kinds of content iterator: preorder and postorder, which can be created via NS_NewPreContentIterator and NS_NewContentIterator respectively. But we don't have a content iterator which visits a parent node at both the start and the end. We could create that, though.

I'm more concerned about other problem like the mapping between the generated content and DOM range.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #2)
> > it might be better to check primary frame's display.
> 
> Probably easier to just check
> frame->IsFrameOfType(nsIFrame::eLineParticipant)

Oh, thanks. I'll check it.

> > Inserting start of non-inline box is easy. However, inserting line break to
> > end of boxes is difficult. I have no idea how to do it with content iterator.
> 
> I'm not sure why inserting at the start is easier. We have two kinds of
> content iterator: preorder and postorder, which can be created via
> NS_NewPreContentIterator and NS_NewContentIterator respectively.

Because ContentEventHandler needs to compute offset from start of the selection root.

> But we don't have a content iterator which visits a parent node at both the start
> and the end. We could create that, though.

Yeah.

> I'm more concerned about other problem like the mapping between the
> generated content and DOM range.

It's out of scope of this bug. Generated contents are not handled normally in Gecko (like selection, find, etc).

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm working on bug 1213589 which is caused by not inserting line breaks at open/close tag of block level elements. So, the patches will fix this bug completely.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I landed the patches for bug 1213589. Therefore, open tag and empty elements cause line breaks in 45. I'll work on inserting line breaks at close tags later.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Resetting assignee which I don't work on in this several months.