Closed Bug 1394758 Opened 8 years ago Closed 8 years ago

Backspace /Delete doesn't delete the first character in ContentEditable=true box if there is another tag inserted in it.

Categories

(Core :: DOM: Editor, defect, P3)

Product:
Core
Component:
DOM: Editor
Version:
57 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: er.vikas, Assigned: m_kato)

References

Details

Attachments

(2 files)

Bug 1394758 - Part 1. non-editable text node should be treated as WSType::special, not WSType::text.
59 bytes, text/x-review-board-request
masayuki
: review+
Details
Bug 1394758 - Part 2. WSRunObject::InsertBreak should convert space to NBSP when current position is first text run object.
59 bytes, text/x-review-board-request
masayuki
: review+
Details
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36 Steps to reproduce: I have Following Structure: <div contenteditable='true'> <span contenteditable='false'>Hello</span> World </div> or <div contenteditable='true'> <span>Hello</span> World </div> I am trying to delete World from this structure using backspace or delete button. I have tested it in various versions from FF 52 - 57 Actual results: I am able to delete all the characters except 'W' (first Char after embedded structure) in above example. Expected results: Backspace/ Delete should be able to delete the whole word ('World') atleast (In perfect world I would like to delete the embedded structure as well i.e. <span> in above example), but in any scenario I am not able to delete first character.
Component: Untriaged → Editor
Product: Firefox → Core
er.vikas, Could you attach completely sample? When I create simple HTML, then I set caret to last character after 'World', I can remove all word correctly. <div contenteditable='true'> <span>Hello</span> World </div>
Flags: needinfo?(er.vikas)
Here is HTML Code which would help you to reproduce this issue, Please let me know if you need more information <html> <body> <div style='padding-left:5rem'> <h3> Try to delete "World" using backspace or delete button: </h3> <div contenteditable='true' style='border:1px solid black; width:30%; height:10rem;'> <span contenteditable="false"> Hello </span> World </div> </div> </body> </html>
Flags: needinfo?(er.vikas)
Thanks, I can reproduce this
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Comment on attachment 8904777 [details] Bug 1394758 - Part 1. non-editable text node should be treated as WSType::special, not WSType::text. https://reviewboard.mozilla.org/r/176320/#review181628 ::: commit-message-1401e:1 (Diff revision 1) > +Bug 1394758 - Part 1. non-editable text node should be WSType::special, not WSType::text. r?masayuki s/should be WSType::special/should be treated as WSType::special ? ::: commit-message-1401e:5 (Diff revision 1) > +Bug 1394758 - Part 1. non-editable text node should be WSType::special, not WSType::text. r?masayuki > + > +This bug occurs that WSRunObject::PrepareToDeleteRangePriv() calls WSRunObject::ConvertToNBSP for non-editable text node. > + > +Actually, even if text node isn't editable, WSFragment::mType becomes WSType::text now. So, tail's whitespace node into contenteditable=false becomes WSType::normalWS that is editable. s/tail's whitespace node into contenteditable=false becomes WSType::normalWS that is editable/whitespace only text node at the end of contenteditable=false is treated as WSType::nomalWS, i.e., treated as editable ? ::: commit-message-1401e:7 (Diff revision 1) > + > +This bug occurs that WSRunObject::PrepareToDeleteRangePriv() calls WSRunObject::ConvertToNBSP for non-editable text node. > + > +Actually, even if text node isn't editable, WSFragment::mType becomes WSType::text now. So, tail's whitespace node into contenteditable=false becomes WSType::normalWS that is editable. > + > +So text node into contenteditable=false should detect as WSType::special that isn't modify the text node and kind of inline. If so, WSRunObject doesn't create object chunk of WSType::normalWS for text node into contenteditable=false. s/text node into contenteditable=false should detect as WSType::special that isn't modify the text node and kind of inline/text nodes in contenteditable=false should be treated as WSType::special which means a non-editable inline object s/If so, WSRunObject doesn't create/Then, WSRunObject won't create s/text node into contenteditable=false/text node in contenteditable=false ? ::: editor/libeditor/tests/test_bug1394758.html:36 (Diff revision 1) > + synthesizeKey("VK_RIGHT", {}); > + synthesizeKey("VK_RIGHT", {}); > + synthesizeKey("VK_RIGHT", {}); > + synthesizeKey("VK_BACK_SPACE", {}); > + synthesizeKey("VK_BACK_SPACE", {}); Could you use "KEY_ArrowRight" and "KEY_Backspace" instead of "VK_RIGHT" and "VK_BACK_SPACE"? ::: editor/libeditor/tests/test_bug1394758.html:47 (Diff revision 1) > + is(span.textContent, beforeSpan, > + "VK_BACK_SPACE should not modify non-editable area"); > + is(span.nextSibling.textContent.trim(), "rld", > + "VK_BACK_SPACE should delete first 2 characters"); > + > + synthesizeKey("VK_DELETE", {}); Could you use "KEY_Delete" instead of "VK_DELETE"?
Attachment #8904777 - Flags: review?(masayuki) → review+
Comment on attachment 8904778 [details] Bug 1394758 - Part 2. WSRunObject::InsertBreak should convert space to NBSP when current position is first text run object. https://reviewboard.mozilla.org/r/176322/#review181630 ::: commit-message-1bd80:9 (Diff revision 1) > + > +1. content is <span contenteditable=false>A</span>[caret] ; > +2. [VK_RETURN] > +3. content is <span contenteditable=false>A</span><br>; <- whitespace is removed > + > +Since readonly text node is WSType::special by part 1, WSRunObject::InsertBreak doesn't convert space (after caret) to NBSP. Because WSRunObject::InsertBreak doesn't consider when inserted position is first text run object. s/Since readonly text node is WSType::special by part 1/Since we started to treat readonly text nodes as WSType::special with the previous patch s/to NBSP. Because WSRunObject::InsertBreak doesn't consider when inserted position is first/to NBSP because WSRunObject::InsertBreak does it only when inserted position is not first ?
Attachment #8904778 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/adcfea5cd531 Part 1. non-editable text node should be treated as WSType::special, not WSType::text. r=masayuki https://hg.mozilla.org/integration/autoland/rev/c65dfa68df8d Part 2. WSRunObject::InsertBreak should convert space to NBSP when current position is first text run object. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/adcfea5cd531 https://hg.mozilla.org/mozilla-central/rev/c65dfa68df8d
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
See Also: → 1665167
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: