Editor makes bogus assumptions about frame offsets

NEW
Unassigned

Status

()

11 years ago
9 years ago

People

(Reporter: bzbarsky, Unassigned)

Tracking

Trunk
Points:
---
Bug Flags:
wanted-next +
blocking1.9 -
wanted1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

11 years ago
nsEditor::EndPlaceHolderTransaction has this comment:

   // By making the assumption that no reflow happens during the calls
   // to EndUpdateViewBatch and ScrollSelectionIntoView, we are able to
   // allow the selection to cache a frame offset which is used by the
   // caret drawing code.

Unfortunately, EndUpdateViewBatch can most certainly trigger reflow.  So this comment is just bogus.  Which makes me very suspicious of the code...

Do we actually need this cache anymore?  We only use it for ScrollSelectionIntoView, right?
Flags: blocking1.9?

Comment 1

11 years ago
This stuff was never very perf-sensitive, as far as I know... I'd say blow away the caching code.  GetPointFromOffset isn't so expensive that it's worth the trouble of caching the result.
(Reporter)

Comment 2

11 years ago
The caching was added for bug 35296.  We should make sure to test that if we remove (which I whole heartedly support).
(Reporter)

Comment 4

11 years ago
Presumably incorrect caret placement in some cases.  At least I hope that's the worst that will happen.  If this cached frame stuff doesn't deal with frame destruction under EndUpdateViewBatch (which can easily happen), then we get the usual "accessing deleted frame" stuff.
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
You need to log in before you can comment on or make changes to this bug.