Closed
Bug 179384
Opened 22 years ago
Closed 22 years ago
Merging blocks via forward delete sends selection to front of doc
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
VERIFIED
FIXED
M1
People
(Reporter: mozeditor, Assigned: mozeditor)
References
()
Details
(Whiteboard: editorbase+; fixinhand; need r=, sr=)
Attachments
(1 file)
20.36 KB,
patch
|
cmanske
:
review+
kinmoz
:
superreview+
|
Details | Diff | Splinter Review |
See Jim Booth's good example in the URL above. Originally reported in an unrelated bug, so I opened a new one.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Whiteboard: editorbase
Target Milestone: --- → M1
Assignee | ||
Updated•22 years ago
|
Whiteboard: editorbase → editorbase moose
Assignee | ||
Updated•22 years ago
|
Whiteboard: editorbase moose → editorbase+
Assignee | ||
Comment 1•22 years ago
|
||
*** Bug 179880 has been marked as a duplicate of this bug. ***
Sorry for my dupe (bug 179384). I'm surprised to find this bug listed under "browser" (that's why i didn't find it).
Assignee | ||
Comment 3•22 years ago
|
||
*** Bug 180784 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 4•22 years ago
|
||
The bug fix is actually just a few lines in nsRangeUpdater::SelAdjJoinNodes(), but I did some other work while I was at it. I made nsRangeUpdater::SelAdjDeleteNode() also adjust range items that are in or under the deleted node. I used to not do this becasue I was afraid of the performance impact, but I was probably thinking about examining all descendants of the deleted node. The quick way to do it is simply to look up the parent chain of the range item, and see if it is under the deleted node. In order to made that check, I needed the util routine IsDecendantOf(), which was in nsHTMLEditUtils. nsSelectionState is used in the base editor, so I moved IsDecendantOf() out of nsHTMLEditUtils and into nsEditorUtils. While I was at it I also moved IsLeafNode(), which is another routine that is not html-specific. Then I had to fix up all callers of these routines to use the new class name. So most of the diff is just mechanically moving code around to where it should have been.
Assignee | ||
Updated•22 years ago
|
Whiteboard: editorbase+ → editorbase+; fixinhand; need r=, sr=
Assignee | ||
Updated•22 years ago
|
Attachment #106898 -
Flags: superreview?(kin)
Attachment #106898 -
Flags: review?(brade)
Assignee | ||
Updated•22 years ago
|
Attachment #106898 -
Flags: review?(brade) → review?(cmanske)
Comment on attachment 106898 [details] [diff] [review] patch to libeditor ==== In general the .get() shouldn't be necessary anymore, but you know that. :-) ==== Can we put a blank line before this comment? + nsCOMPtr<nsIDOMNode> oldStart; + // check for range endpoints that are in descendants of aNode + if (nsEditorUtils::IsDescendantOf(item->startNode, aNode)) ==== And this one? + } + // avoid having to call IsDescendantOf() for common case of range startnode == range endnode. + if ((item->endNode == oldStart) || nsEditorUtils::IsDescendantOf(item->endNode, aNode)) ==== And this one? item->endOffset += aOldLeftNodeLength; + // adjust endpoints in aLeftNode + if (item->startNode.get() == aLeftNode)
Attachment #106898 -
Flags: superreview?(kin) → superreview+
Comment 6•22 years ago
|
||
Comment on attachment 106898 [details] [diff] [review] patch to libeditor Looks good. Works well. :)
Attachment #106898 -
Flags: review?(cmanske) → review+
Assignee | ||
Comment 7•22 years ago
|
||
fix landed on trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 8•22 years ago
|
||
Comment on attachment 106898 [details] [diff] [review] patch to libeditor This change have added a warning on brad TBox: +editor/libeditor/base/nsSelectionState.cpp:307 + `struct nsRangeStore * item' might be used uninitialized in this function Indeed, if the count is <=0, then the for loop is never executed and we end up dereferencing an uninitialized pointer! P.S. Per bug 179819 such "uninitalized" warnings ought to be considered compilation errors.
Assignee | ||
Comment 9•22 years ago
|
||
Wow, what a brain fart. All the new lines in that function were supposed to be inside the for loop, not outside. I'm landing a fix for that right now. Good catch!
Comment 10•22 years ago
|
||
*** Bug 170801 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
QA Contact: sujay → sairuh
Comment 11•21 years ago
|
||
vrfy'd fixed on win2k and linux rh8.0 (2003.03.03) and mac 10.2.4 (2003.02.28).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•