Closed
Bug 1394758
Opened 7 years ago
Closed 7 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)
Tracking
()
RESOLVED
FIXED
mozilla57
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 |
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.
Updated•7 years ago
|
Component: Untriaged → Editor
Product: Firefox → Core
Assignee | ||
Comment 1•7 years ago
|
||
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)
Assignee | ||
Comment 3•7 years ago
|
||
Thanks, I can reproduce this
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd2834e4ba4282ae1d40f46c60a0d2a6b521b29a
Assignee: nobody → m_kato
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
mozreview-review |
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 9•7 years ago
|
||
mozreview-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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
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
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/adcfea5cd531 https://hg.mozilla.org/mozilla-central/rev/c65dfa68df8d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•