956032 - Accessible object:text-selection-changed event missing when all selected text becomes unselected

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Attachment: accessible-event listener

Steps to reproduce:
1. Launch the attached accessible-event listener in a terminal
2. View the attached test case in Firefox
3. Place the caret in the textarea then press Ctrl+A to select its contents
4. Press Home

Expected results: After Home is pressed, causing all of the selected text to become unselected, an object:text-selection-changed event would be emitted.

Actual results: When all of the selected text becomes unselected, there is no object:text-selection-changed event.

Notes:
1. This bug is not limited to Ctrl+A followed by Home. Anything that causes the  number of selected characters to go from > 0 to 0 will result in this bug.

2. This bug can also be seen in Thunderbird.

Comment 1

[Joanmarie Diggs]

Attachment: test case: textselection.html

Comment 2

[alexander :surkov (:asurkov)]

What Firefox version? We have fixed bug 688124 in Firefox 27.

Comment 3

[Joanmarie Diggs]

(In reply to alexander :surkov from comment #2)
> What Firefox version? We have fixed bug 688124 in Firefox 27.

I can reproduce this in 29.0a1 (2014-01-02).

Comment 4

[alexander :surkov (:asurkov)]

ok, confirming

Comment 5

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #5)
> Created attachment 8356738 [details] [diff] [review]
> patch

wouldn't it be nicer to improve the selection observer interface so we don't need to figure this out again? I'm sure it already knows this...

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #6)
> (In reply to alexander :surkov from comment #5)
> > Created attachment 8356738 [details] [diff] [review]
> > patch
> 
> wouldn't it be nicer to improve the selection observer interface so we don't
> need to figure this out again? I'm sure it already knows this...

from my point of view it will be but it might be a big change for pure accessibility needs, in general we are asked to keep stuffs on our side. but let's cc Olli to see what he thinks.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

notifySelectionChanged does tell you about the selection change?

Comment 9

[alexander :surkov (:asurkov)]

(In reply to Olli Pettay [:smaug] from comment #8)
> notifySelectionChanged does tell you about the selection change?

yes but we need to know how the selection was changed, in particular, if it was collapsed or not.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

What |reason| do you get when notifySelectionChanged is called?

Comment 11

[alexander :surkov (:asurkov)]

(In reply to Olli Pettay [:smaug] from comment #10)
> What |reason| do you get when notifySelectionChanged is called?

it's zero, can be 4 or 8 pointing keyboard/mouse presence, but no those collapse things

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Hmm, sounds like we should just fix the reason, especially if we do collapse to the start or end.
Stack trace for the notifySelectionChanged would be useful.

Comment 13

[alexander :surkov (:asurkov)]

the stack is (pressing left arrow on all selected text in textarea):

Selection changed, selection type: normal, notification pending, reason: 0
  stack: 
mozilla::a11y::logging::SelChange(nsISelection*, mozilla::a11y::DocAccessible*, short)+0x00000135 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0338d325]
mozilla::a11y::SelectionManager::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short)+0x000000e6 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0338f556]
mozilla::Selection::NotifySelectionListeners()+0x000001a0 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc8810]
nsFrameSelection::NotifySelectionListeners(short)+0x0000008b [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc779b]
mozilla::Selection::Collapse(nsINode&, unsigned int, mozilla::ErrorResult&)+0x00000315 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dd0e45]
mozilla::Selection::Collapse(nsINode*, int)+0x00000052 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc3282]
nsFrameSelection::MoveCaret(unsigned int, bool, nsSelectionAmount, bool)+0x000003d3 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc2743]
nsFrameSelection::MoveCaret(unsigned int, bool, nsSelectionAmount)+0x000000de [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc235e]
nsFrameSelection::CharacterMove(bool, bool)+0x0000006c [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02dc812c]
nsTextInputSelectionImpl::CharacterMove(bool, bool)+0x0000006e [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x025c9eae]
non-virtual thunk to nsTextInputSelectionImpl::CharacterMove(bool, bool)+0x0000003d [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x025c9f0d]
nsSelectionMoveCommands::DoCommand(char const*, nsISupports*)+0x0000069e [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x029753fe]
nsControllerCommandTable::DoCommand(char const*, nsISupports*)+0x000000f2 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0331e8d2]
nsBaseCommandController::DoCommand(char const*)+0x00000182 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x03318e62]
DoCommandCallback(char const*, void*)+0x0000035d [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x025cc21d]
mozilla::widget::NativeKeyBindings::KeyPress(mozilla::WidgetKeyboardEvent const&, void (*)(char const*, void*), void*)+0x00000592 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a2e622]
nsTextInputListener::HandleEvent(nsIDOMEvent*)+0x000002e2 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x025cbd52]
non-virtual thunk to nsTextInputListener::HandleEvent(nsIDOMEvent*)+0x0000002f [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x025cc29f]
nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEvent*, mozilla::dom::EventTarget*)+0x0000017b [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024c471b]
nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*)+0x000002c2 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024c4a32]
nsEventListenerManager::HandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*)+0x00000159 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024d86e9]
nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&)+0x000001ed [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024cf68d]
nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&)+0x00000275 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024bd8c5]
nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&)+0x0000056c [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024bdbbc]
nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*)+0x000012d9 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x024bf0e9]
PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*)+0x00001026 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02c0d166]
PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*)+0x00001f65 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02c0bd05]
PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*)+0x000003db [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02c0a17b]
nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*)+0x00000281 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02189971]
nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool)+0x0000013f [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x02186b4f]
nsChildView::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&)+0x000004e7 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x019ebec7]
nsChildView::DispatchWindowEvent(mozilla::WidgetGUIEvent&)+0x00000036 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x019ebf76]
mozilla::widget::TextInputHandlerBase::DispatchEvent(mozilla::WidgetGUIEvent&)+0x000000b9 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a1b489]
mozilla::widget::TextInputHandler::DoCommandBySelector(char const*)+0x000002e6 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a1fe26]
-[ChildView doCommandBySelector:]+0x0000006a [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x019fdffa]
-[NSKeyBindingManager(NSKeyBindingManager_MultiClients) interpretEventAsCommand:forClient:]+0x00000570 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x0025b2d1]
-[NSTextInputContext handleEvent:]+0x0000034d [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x0027a322]
-[NSView interpretKeyEvents:]+0x000000b4 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x00259b5d]
mozilla::widget::TextInputHandler::HandleKeyDownEvent(NSEvent*)+0x00000966 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a1a966]
-[ChildView keyDown:]+0x000009a3 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x019ffd83]
-[NSWindow sendEvent:]+0x00000733 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x0022681b]
-[ToolbarWindow sendEvent:]+0x000001ac [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a46d1c]
-[NSApplication sendEvent:]+0x00000d43 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x001c7ca2]
-[GeckoNSApplication sendEvent:]+0x00000092 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a33572]
-[NSApplication run]+0x00000286 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x00017a29]
nsAppShell::Run()+0x00000091 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01a35191]
nsAppStartup::Run()+0x0000009c [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x036c341c]
XREMain::XRE_mainRun()+0x00001879 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0357edf9]
XREMain::XRE_main(int, char**, nsXREAppData const*)+0x000002b9 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0357f5f9]
XRE_main+0x0000004d [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x0357fa6d]
do_main(int, char**, nsIFile*)+0x00000667 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/firefox +0x00002087]
main+0x00000141 [/Users/alex/mozilla/0411/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/firefox +0x000015c1]
  sync notification processing

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Oh, I didn't give feedback :/ I'm pretty sure I wrote it.

So we should go up from that stack to look at what kind of reason can be given, and if 
COLLAPSETOSTART_REASON or COLLAPSETOEND_REASON don't quite fit, we could add a new reason, 
Something like COLLAPSEALL_REASON or such

Comment 15

[alexander :surkov (:asurkov)]

We could call Selection::CollapseToStart/CollapseToEnd in nsFrameSelection::MoveCaret (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp#795) or add PostReason there instead? Neither of ways is elegant, preferences?

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

I guess PostReason would work.

Comment 17

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 18

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8370925 [details] [diff] [review]
patch2

I hate how much machinary we're adding here, but I'm not sure I have a better idea.

>+AccTextSelChangeEvent::IsNotCaretMoveOnly() const

this name involves two many negations for my small brain, how about making it IsCaretMoveOnly()?

>+{
>+  return mSel->GetRangeCount() != 1 || !mSel->IsCollapsed() ||
>+    mReason & (nsISelectionListener::COLLAPSETOSTART_REASON |
>+               nsISelectionListener::COLLAPSETOSTART_REASON);

presumably one of those should be COLAPSETOEND?

>+struct mozilla::a11y::SelNReason

what does SelNReason mean its kind of funny name. maybe SelData?

>+  NS_INLINE_DECL_REFCOUNTING(SelNReason);

refcounting this is pretty silly...

r=me with the naming stuff fixed up

Comment 19

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/5290d9ce7f16

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5290d9ce7f16