calling GetAnchor/GetFocus from editor code

VERIFIED FIXED in M17

Status

()

P3
major
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: buster, Assigned: mozeditor)

Tracking

Trunk
Other
Other
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
From Joe's posting:
We shouldn't ever be calling GetFocusNode()/GetAnchorNode() in the
editor.  I
wasn't going to say anything about it until I found an actual
example of code
that was broken because of this approach.  Well, I've
found it: the old
nsTextEditRules::DidUndo()/DidRedo() code didn't work
at all and caused
flurries of bogus nodes to start populating the
document.  There are 6
(remaining) places where we call GetFocusNode()
and 17 places where we call
GetAnchorNode() and I consider them all
guilty until proven innocent.  The
editor never cares, as far as I can
tell, what the selection code considers
anchor or focus, and our
operations should instead just operate on selection
ranges.
(Reporter)

Updated

19 years ago
Summary: calling GetAnchor/GetFocus from editor code

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M12

Comment 1

19 years ago
I will review all uses of this, although I don't understand how it affects
undo.
(Reporter)

Comment 2

19 years ago
kin or I can help with any undo questions.

Comment 3

19 years ago
Can someone explain more about how using it generates "bogus nodes"?
The cases that use GetFocusNode() seem to be valid:
E.g., nsAutoSelectionReset uses them to save the current selection endpoints.
Steve: Do you want me to evaluate their use in code I didn't write?
Their use in SetTextPropertiesForNodeWithDifferentParents looks suspicious to
me.

Comment 4

19 years ago
Steve: Please evaluate SetTextPropertiesForNodeWithDifferentParents as
requested above.

Updated

19 years ago
Summary: calling GetAnchor/GetFocus from editor code → [CODE cleanup]calling GetAnchor/GetFocus from editor code
Whiteboard: [CODE general cleanup]
Target Milestone: M12 → M17

Comment 5

19 years ago
setting out to M17, general code cleanup

Updated

19 years ago
Assignee: cmanske → jfrancis
Status: ASSIGNED → NEW

Comment 6

19 years ago
The instances in my code are Ok.
I'm not sure about those in other people's code.
Joe: please review uses in your code.
(Assignee)

Comment 7

19 years ago
accepting bug

Comment 8

19 years ago
removing whiteboard status text
Summary: [CODE cleanup]calling GetAnchor/GetFocus from editor code → calling GetAnchor/GetFocus from editor code
Whiteboard: [CODE general cleanup]
(Assignee)

Comment 9

19 years ago
All this stuff has been reviewed, so I'm closing it out.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 10

19 years ago
verified in 4/25 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.