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)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: surkov, Assigned: surkov)
Details
Attachments
(1 file)
1.62 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter 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)
Updated•8 years ago
|
Assignee: nobody → surkov.alexander
Comment 1•8 years ago
|
||
Comment on attachment 8724446 [details] [diff] [review] patch Review of attachment 8724446 [details] [diff] [review]: ----------------------------------------------------------------- I don't really understand this code.
Assignee | ||
Comment 2•8 years ago
|
||
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 3•8 years ago
|
||
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+
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8282634edb31
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ea750d9b568
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0a6bc0d0062
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cdc1b0f32b709b0e9357961d8aa790ba9f53cd7 Bug 1251871 - text removal from a hypertext may result in empty text nodes, r=davidb
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4cdc1b0f32b7
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•