Closed Bug 460740 Opened 16 years ago Closed 13 years ago

[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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: dpopa, Assigned: kaze)

References

Details

(Keywords: testcase)

Attachments

(4 files, 3 obsolete files)

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
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.
Bug 647760 claims nothing happens when pressing Enter in the second case
on Windows 7.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
Summary: [contenteditable] 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 contenteditable LI or 2. a contenteditable element inside a LI or 3. a contenteditable element inside a block element inside a LI
(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.
Assignee: nobody → ehsan
Whiteboard: [post-2.0]
I guess the reason behind this bug is missing to check for the active editing host somewhere...
Assignee: ehsan → 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.
Here's a testcase that adds three tests involving paragraphs.
Attached patch patch proposal (obsolete) — Splinter Review
Here’s a quick fix, I still have to write the tests.
Couldn't you use IsNodeInActiveEditor?
(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.
Status: NEW → ASSIGNED
Attached patch patch proposal (obsolete) — Splinter Review
Same patch, with tests.
Attachment #548213 - Attachment is obsolete: true
Attachment #548426 - Flags: review?
(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 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.
Attachment #548426 - Flags: review?
Attached patch patch proposal (obsolete) — Splinter Review
Same patch, updated tests.
Attachment #548426 - Attachment is obsolete: true
Attachment #548459 - Flags: review?(ehsan)
Attached patch patch proposalSplinter Review
s/kimpleTest/SimpleTest/
sorry for the typo :-s
Attachment #548459 - Attachment is obsolete: true
Attachment #548523 - Flags: review?(ehsan)
Attachment #548459 - Flags: review?(ehsan)
Attachment #548523 - Flags: review?(ehsan) → review+
Whiteboard: [post-2.0]
Pushed to mozilla-inbound.
http://hg.mozilla.org/mozilla-central/rev/d035ba160ce0
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Depends on: 787434
No longer depends on: 787434
You need to log in before you can comment on or make changes to this bug.