Last Comment Bug 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
: [contenteditable][list-item] invalid results when pressing ENTER inside 1. a ...
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: unspecified
: All All
: -- normal with 3 votes (vote)
: mozilla8
Assigned To: Fabien Cazenave [:kaze]
:
:
Mentors:
: 389342 462028 597349 647760 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-20 05:09 PDT by Dan POPA
Modified: 2012-08-31 18:02 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase: editable LI, editable DIV inside LI, editable DIV inside DIV inside LI (1.22 KB, text/html)
2008-10-20 05:12 PDT, Dan POPA
no flags Details
contenteditable breaks inside list item demo (685 bytes, text/html)
2009-03-31 11:00 PDT, Michael
no flags Details
testcase: [contentEditable] invalid results when pressing Enter (1.47 KB, text/html)
2011-07-25 10:12 PDT, Fabien Cazenave [:kaze]
no flags Details
patch proposal (2.77 KB, patch)
2011-07-25 10:16 PDT, Fabien Cazenave [:kaze]
no flags Details | Diff | Splinter Review
patch proposal (6.43 KB, patch)
2011-07-26 05:46 PDT, Fabien Cazenave [:kaze]
no flags Details | Diff | Splinter Review
patch proposal (6.77 KB, patch)
2011-07-26 07:59 PDT, Fabien Cazenave [:kaze]
no flags Details | Diff | Splinter Review
patch proposal (6.40 KB, patch)
2011-07-26 10:52 PDT, Fabien Cazenave [:kaze]
ehsan: review+
Details | Diff | Splinter Review

Description Dan POPA 2008-10-20 05:09:49 PDT
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 2008-10-20 05:12:37 PDT
Created attachment 343883 [details]
testcase: editable LI, editable DIV inside LI, editable DIV inside DIV inside LI
Comment 2 Michael 2009-03-31 10:59:02 PDT
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 2009-03-31 11:00:08 PDT
Created attachment 370233 [details]
contenteditable breaks inside list item demo
Comment 4 Mats Palmgren (:mats) 2011-04-05 12:47:44 PDT
*** Bug 647760 has been marked as a duplicate of this bug. ***
Comment 5 Mats Palmgren (:mats) 2011-04-05 12:50:43 PDT
Bug 647760 claims nothing happens when pressing Enter in the second case
on Windows 7.
Comment 6 Dan POPA 2011-04-06 00:39:53 PDT
(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 :Ehsan Akhgari 2011-07-21 15:52:38 PDT
I guess the reason behind this bug is missing to check for the active editing host somewhere...
Comment 8 Fabien Cazenave [:kaze] 2011-07-25 07:03:23 PDT
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] 2011-07-25 10:12:57 PDT
Created attachment 548212 [details]
testcase: [contentEditable] invalid results when pressing Enter

Here's a testcase that adds three tests involving paragraphs.
Comment 10 Fabien Cazenave [:kaze] 2011-07-25 10:16:14 PDT
Created attachment 548213 [details] [diff] [review]
patch proposal

Here’s a quick fix, I still have to write the tests.
Comment 11 :Ehsan Akhgari 2011-07-25 20:14:56 PDT
Couldn't you use IsNodeInActiveEditor?
Comment 12 Fabien Cazenave [:kaze] 2011-07-26 05:05:06 PDT
(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] 2011-07-26 05:46:17 PDT
Created attachment 548426 [details] [diff] [review]
patch proposal

Same patch, with tests.
Comment 14 :Ehsan Akhgari 2011-07-26 06:45:16 PDT
(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 :Ehsan Akhgari 2011-07-26 06:51:14 PDT
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] 2011-07-26 07:59:50 PDT
Created attachment 548459 [details] [diff] [review]
patch proposal

Same patch, updated tests.
Comment 17 Fabien Cazenave [:kaze] 2011-07-26 10:52:25 PDT
Created attachment 548523 [details] [diff] [review]
patch proposal

s/kimpleTest/SimpleTest/
sorry for the typo :-s
Comment 18 :Ehsan Akhgari 2011-07-26 14:37:25 PDT
Pushed to mozilla-inbound.
Comment 19 Marco Bonardo [::mak] 2011-07-27 03:41:48 PDT
http://hg.mozilla.org/mozilla-central/rev/d035ba160ce0
Comment 20 Fabien Cazenave [:kaze] 2011-07-28 03:30:29 PDT
*** Bug 389342 has been marked as a duplicate of this bug. ***
Comment 21 Fabien Cazenave [:kaze] 2011-08-24 03:18:55 PDT
*** Bug 597349 has been marked as a duplicate of this bug. ***
Comment 22 Fabien Cazenave [:kaze] 2011-08-24 03:53:53 PDT
*** Bug 462028 has been marked as a duplicate of this bug. ***

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