Closed Bug 414526 Opened 12 years ago Closed 9 years ago
[contenteditable] DELETE & BACKSPACE when selection at the beginning or the end of a contenteditable element affects the adjacent contenteditable elements
BACKSPACE touches once the contenteditable element before current one and removes characters from the end of the first child of this contenteditable element.
5.17 KB, text/html
DELETE key inside an contenteditable element affects content in the next adjacent contenteditable element.
4.45 KB, text/html
DELETE key removes invalid content (or it doesn't) when selection is inside/over a BR located before some element inside an contenteditable region.
3.91 KB, text/html
2.77 KB, text/html
2.56 KB, text/html
testcase showing also other problems (with delete key, replacing selection, caret display and position)
2.82 KB, text/html
35.15 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012804 Minefield/3.0b3pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012804 Minefield/3.0b3pre DELETE & BACKSPACE when selection at the beginning or the end of a contenteditable element affects the adjacent contenteditable elements. Reproduction steps needs verbose description for each case, so I will add an attachment for each case. Attachments may also be usefully for debugging as they contain an event log for the mutation events fired on the document while editing. Reproducible: Always
This bug is related to bug 332635. Asking for the blocking1.9 flag.
Comment on attachment 299976 [details] BACKSPACE touches once the contenteditable element before current one and removes characters from the end of the first child of this contenteditable element Marking first attachment obsolete, will be replaced with an updated version.
Attachment #299976 - Attachment is obsolete: true
Re-added first attachment, found more details on how to duplicate.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Adding PeterV to CC, PeterV, what do you think about this? Firefox 2 suffers from this, though only in designMode, so this isn't a regression. In the testcase <div contenteditable="true"><br><ul><li>something</li></ul></div>, if you delete the <br>, it removes the next <li>'s text from the <ul>, and renders that text below the <ul>. The problem is the call to nsHTMLEditor::IsVisBreak() ( http://mxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLEditor.cpp#1141 ) in nsHTMLEditRules::WillDeleteSelection(): http://mxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLEditRules.cpp#2015 nsHTMLEditor::IsVisBreak() is returning false, so we're assuming that deleting the <br> it will have no effect, so we delete it, then carry on and delete the next thing or some such. The assumption by IsVisBreak() is that a <br> next to a block node is invisible, but that assumption is not always true. It should be a <br> before a block node is only visible when the <br> has only non-visible whitespace before it, otherwise it's invisible. I've discussed this with Roc, and his suggestion is that we don't do things this way. We're trying to figure out if deleting the node at the caret will cause a layout change. He suggests instead of trying to figure out what layout will do, we just go ahead and delete the node, flush layout, and figure out if the layout actually changed. This would not be a trivial change, as IsVisBreak() is used a number of places in the editor code, so that would have to be wanted-next. In the meantime I could hack IsVisBreak() to kinda do what we want... What do people think?
There are more problems here too, we're allowing the delete to propagate outside of the contenteditable area, maybe we should check that the node-being-deleted is a descendant of the editable root.
find another broken cases with backspace, can be reproduced with FF 3.0.4 and FF3.1beta1 20081118 nighly build, please open this attached file and follow the on screen instruction
I think in ff 3.6 the bug is closed...
Only the cases described in attachment 299983 [details] (second attachment) seems to be fixed. I can reproduce all others in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100310 Minefield/3.7a3pre
Duplicate of this bug: 506661
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Should this be reviewed?
Wait a couple of days. I'm writing tests and I should test it on tryserver.
Masayuki: are you planning to continue working on this?
Ehsan: Sorry for the delay. I'm checking the state of latest patch which is in my local. But I don't have much time for now, I hope somebody continues to work for this bug after my next post.
OK, no problem. Please send the latest version of the patch that you have, and I'll start working on it.
This is the latest version. > 208 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_bug414526.html | Pressing backspace key at end of editor3 changes the content - got "<p id=\"editor1\" contenteditable=\"true\">editor1</p><p id=\"editor2\" contenteditable=\"true\">editor2</p><div id=\"editor3\" contenteditable=\"true\"><div>editor3editor4editor5</div></div><p id=\"editor4\" contenteditable=\"true\"></p>non-editable text<p id=\"editor5\" contenteditable=\"true\"></p>", expected "<p id=\"editor1\" contenteditable=\"true\">editor1</p><p id=\"editor2\" contenteditable=\"true\">editor2</p><div id=\"editor3\" contenteditable=\"true\"><div>editor3</div></div><p id=\"editor4\" contenteditable=\"true\">editor4</p>non-editable text<p id=\"editor5\" contenteditable=\"true\">editor5</p>" > 472 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_bug551704.html | test5: The expected HTML after editing should be correct - got "abc<br>def", expected "abc<br>def<br>" > 475 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_bug551704.html | test6: The expected HTML after editing should be correct - got "abc<br>def", expected "abc<br>def<br>" This still fails backspace key handling in editor3 of new tests when caret is at the end of the editor3. And also, this patch breaks <br> node insertion at the end of an HTML editor. The main concept of this patch is, HTML editor must check whether all found nodes are in current focused editor or not. If found node is in another editing host, the handling should be canceled.
Attachment #464886 - Attachment is obsolete: true
Note that this patch only fixes the bugs in these testcases which affect the contents of other contenteditable elements on the page. Other bugs should be filed for problems that happen in the testcases only inside a single contenteditable element.
Comment on attachment 545916 [details] [diff] [review] Patch (v4) Review of attachment 545916 [details] [diff] [review]: ----------------------------------------------------------------- ::: editor/libeditor/base/nsEditor.cpp @@ +3801,5 @@ > } > + > +/////////////////////////////////////////////////////////////////////////// > +// GetNodeAtRangeOffsetPoint: returns the node at this position in a range, > +// assuming that aParentOrNode is the node itself it if's a text node, or if it's
Attachment #545916 - Flags: review?(roc) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.