If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Editor makes bogus assumptions about frame offsets

NEW
Unassigned

Status

()

Core
Editor
10 years ago
8 years ago

People

(Reporter: bz, Unassigned)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

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

10 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.
The caching was added for bug 35296.  We should make sure to test that if we remove (which I whole heartedly support).
What symptoms does this have?
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: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
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.