14617 - calling GetAnchor/GetFocus from editor code

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[buster]

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.

Comment 1

[Charles Manske]

I will review all uses of this, although I don't understand how it affects
undo.

Comment 2

[buster]

kin or I can help with any undo questions.

Comment 3

[Charles Manske]

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

[Charles Manske]

Steve: Please evaluate SetTextPropertiesForNodeWithDifferentParents as
requested above.

Comment 5

[rubydoo123]

setting out to M17, general code cleanup

Comment 6

[Charles Manske]

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.

Comment 7

[Joe Francis]

accepting bug

Comment 8

[rubydoo123]

removing whiteboard status text

Comment 9

[Joe Francis]

All this stuff has been reviewed, so I'm closing it out.

Comment 10

[sujay]

verified in 4/25 build.