Closed Bug 1394758 Opened 2 years ago Closed 2 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)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: er.vikas, Assigned: m_kato)

References

Details

Attachments

(2 files)

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
Duplicate of this bug: 685445
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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.