1193988 - nsFrameSelection::TakeFocus notifies listeners twice on some branches

Status: RESOLVED WONTFIX

(Core :: DOM: Selection, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

In the last branch of nsFrameSelection::TakeFocus, Selection::Extend is called. Selection::Extend handles calling NotifySelectionListeners() when the selection is changed, however nsFrameSelection::TakeFocus will also call it after Extend returns.

The function should be changed such that NotifySelectionListeners() is only called once when this branch is taken.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Don't double-notify selection listeners in nsFrameSelection::TakeFocus

Comment 2

[(no longer active)]

This needs a test too.  :)

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to (Ask others for review please; out most of the week) from comment #2)
> This needs a test too.  :)
Indeed it does! I'm currently having a bunch of issues with us notifying selection listeners very overzealously, so I haven't really finalized this patch yet.

For example, we fire a NotifySelectionListener every time you move your selection a pixel, even if it doesn't change the actual range which you selected.

Comment 4

[(no longer active)]

Do we still need this, given your new approach?  If not, I prefer to not change the behavior here.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Nope, we don't. I was thinking of closing this but wanted to talk to you first.