Closed Bug 1251871 Opened 8 years ago Closed 8 years ago

text removal from a hypertext may result in empty text nodes

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: surkov, Assigned: surkov)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
I saw the behavior in tests failures, we select text inside a text node by setting proper DOM ranges and then ask the editor for removal, and then the empty text node stays. So if we are about to select whole text in a text node then select a text node instead.
Attachment #8724446 - Flags: review?(dbolter)
Assignee: nobody → surkov.alexander
Comment on attachment 8724446 [details] [diff] [review]
patch

Review of attachment 8724446 [details] [diff] [review]:
-----------------------------------------------------------------

I don't really understand this code.
this code is conversion of hypertext offest to DOM offset. One singe ht offset may be described by different ways. Say you have <p>hello</p>, then 5th ht offset (offset pointing right after 'o') may be described as (p.firstChild, 5) or (p, 1). Then now we convert ht offsets as (text_node, offset_inside_text_node), and editor.Delete() expectedly remove text node content but it doesn't remove a text node itself, so we may end up with empty text node, which it seems no one need. So my suggestion is no content - no node.
Comment on attachment 8724446 [details] [diff] [review]
patch

Review of attachment 8724446 [details] [diff] [review]:
-----------------------------------------------------------------

OK for a few moments I actually understood this change so while I'm still in that magical world I'll r+, as long as you add a comment making it clear when we expect that new 'if' to be false.
Attachment #8724446 - Flags: review?(dbolter) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cdc1b0f32b709b0e9357961d8aa790ba9f53cd7
Bug 1251871 - text removal from a hypertext may result in empty text nodes, r=davidb
https://hg.mozilla.org/mozilla-central/rev/4cdc1b0f32b7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.