460740 - [contenteditable][list-item] invalid results when pressing ENTER inside 1. a contenteditable LI or 2. a contenteditable element inside a LI or 3. a contenteditable element inside a block element inside a LI

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Dan POPA]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3

[contenteditable] invalid results when pressing ENTER inside 
1. a contenteditable LI (<li contenteditable="true">)
2. a contenteditable element inside a LI (<li><div contenteditable="true">)
3. a contenteditable element inside a block element inside a LI(<li><div><div contenteditable="true">)

Editing should be contained inside the contenteditable element.

Actual results for each case:
1. New contenteditable LI gets created.
2. New contenteditable element gets created.
3. Nothing happens.

Expected results: editing takes place inside the editanle element and:
1. Split selection with a BR.
2. same as 1
3. same as 1

The described expected results are debatable.

Reproducible: Always

Comment 1

[Dan POPA]

Attachment: testcase: editable LI, editable DIV inside LI, editable DIV inside DIV inside LI

Comment 2

[Michael]

I have also encountered this bug in firefox 3.0.8 on both mac and windows. Any way I attempt to structure the document, the same result happens. This does not occur in any other browser.

Comment 3

[Michael]

Attachment: contenteditable breaks inside list item demo

Comment 5

[Mats Palmgren (inactive)]

Bug 647760 claims nothing happens when pressing Enter in the second case
on Windows 7.

Comment 6

[Dan POPA]

(In reply to comment #5)
> Bug 647760 claims nothing happens when pressing Enter in the second case
> on Windows 7.

Nothing happens indeed in the second case but only when the DIV is empty (no contents in it) as it's the case in bug 647760: <ul><li><div
contentEditable=true></div></li></ul>

So bug 647760 is not actually a duplicate of this but in fact another case for this bug.

Comment 7

[(no longer active)]

I guess the reason behind this bug is missing to check for the active editing host somewhere...

Comment 8

[Fabien Cazenave [:kaze]]

Confirmed. Here are two another typical cases where [Enter] should create a <br>:

<p contenteditable>
  Editable P: [Enter] creates another editable P.
</p>
<p>
  <span contenteditable> Editable SPAN in a P:
  [Enter] does nothing. </span>
</p>

I’ll have a look at `nsHTMLEditRules::WillInsertBreak' ASAP.

Comment 9

[Fabien Cazenave [:kaze]]

Attachment: testcase: [contentEditable] invalid results when pressing Enter

Here's a testcase that adds three tests involving paragraphs.

Comment 10

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Here’s a quick fix, I still have to write the tests.

Comment 11

[(no longer active)]

Couldn't you use IsNodeInActiveEditor?

Comment 12

[Fabien Cazenave [:kaze]]

(In reply to comment #11)
> Couldn't you use IsNodeInActiveEditor?

I could.

As IsNodeInActiveEditor(aNode) returns true if `aNode' is the active editing host, we still have to call GetActiveEditingHost() to make sure that `aNode' is not the editing host itself to avoid the duplication of an editable paragraph / listitem / header.

Since nsEditorUtils::IsDescendentOf(aNode, aParent) returns false if aNode==aParent, it seems a little simpler to use IsDescendentOf + GetActiveEditingHost imho. If you prefer using IsNodeInActiveEditor please let me know and I’ll adapt this patch.

Comment 13

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Same patch, with tests.

Comment 14

[(no longer active)]

(In reply to comment #12)
> (In reply to comment #11)
> > Couldn't you use IsNodeInActiveEditor?
> 
> I could.
> 
> As IsNodeInActiveEditor(aNode) returns true if `aNode' is the active editing
> host, we still have to call GetActiveEditingHost() to make sure that `aNode'
> is not the editing host itself to avoid the duplication of an editable
> paragraph / listitem / header.
> 
> Since nsEditorUtils::IsDescendentOf(aNode, aParent) returns false if
> aNode==aParent, it seems a little simpler to use IsDescendentOf +
> GetActiveEditingHost imho. If you prefer using IsNodeInActiveEditor please
> let me know and I’ll adapt this patch.

No, nevermind then!

Comment 15

[(no longer active)]

Comment on attachment 548426 [details] [diff] [review]
patch proposal

The patch looks good, but I think we should also test the behavior of pressing enter at the beginning and in the middle of the text for each element.

Comment 16

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Same patch, updated tests.

Comment 17

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

s/kimpleTest/SimpleTest/
sorry for the typo :-s

Comment 18

[(no longer active)]

Pushed to mozilla-inbound.

Comment 19

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/d035ba160ce0