1429870 - tab-selection accessibility events not always emitted

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[Samuel Thibault]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20180105065505

Steps to reproduce:

Switch between tabs with control-page up / down


Actual results:

Events
interface=org.a11y.atspi.Event.Object; member=StateChanged string "selected"
for the newly selected tab are not always emitted: they get emitted when using control-pagedown, except when arriving on the last tab. Conversely, they never get emitted when using control-pageup, except when arriving on the first tab.


Expected results:

Events
interface=org.a11y.atspi.Event.Object; member=StateChanged string "selected"
for the newly selected tab should always be emitted

Comment 1

[Samuel Thibault]

This also happens with firefox 57.0.1, I'm however here using the firefox 52 source code.

My findings investigating the issue when using control-pageup are as the following:

- in ./browser/base/content/tabbrowser.xml, in the setter of the _selected property of tabbrowser-tab, this.setAttribute is properly always called, and are properly queued with the following backtrace:

#1  0x00007fffeb4cd25c in mozilla::a11y::DocAccessible::AttributeChangedImpl(mozilla::a11y::Accessible*, int, nsIAtom*) (this=this@entry=0x7fffc9831e40, aAccessible=aAccessible@entry=0x7fffc52ba140, aNameSpaceID=aNameSpaceID@entry=0, aAttribute=aAttribute@entry=0x7fffdd7a5e60)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/DocAccessible.cpp:926
#2  0x00007fffeb4cde7b in mozilla::a11y::DocAccessible::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (this=0x7fffc9831e40, aDocument=<optimized out>, aElement=0x7fffca0dca60, aNameSpaceID=0, aAttribute=0x7fffdd7a5e60, aModType=2, aOldValue=0x7fffffff8638)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/DocAccessible.cpp:793
#3  0x00007fffea5bc3b6 in nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (aElement=aElement@entry=0x7fffca0dca60, aNameSpaceID=aNameSpaceID@entry=0, aAttribute=aAttribute@entry=0x7fffdd7a5e60, aModType=aModType@entry=2, aOldValue=aOldValue@entry=0x7fffffff8638)
    at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsNodeUtils.cpp:145
#4  0x00007fffea519202 in mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) (this=this@entry=0x7fffca0dca60, aNamespaceID=aNamespaceID@entry=0, aName=aName@entry=0x7fffdd7a5e60, aPrefix=aPrefix@entry=0x0, aOldValue=..., aParsedValue=..., aModType=2 '\002', aFireMutation=false, aNotify=true, aCallAfterSetAttr=true) at /var/tmp/firefox-esr-52.5.0esr/dom/base/Element.cpp:2523
#5  0x00007fffea5195cb in mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) (this=0x7fffca0dca60, aNamespaceID=0, aName=0x7fffdd7a5e60, aPrefix=0x0, aValue=..., aNotify=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/dom/base/Element.cpp:2386
#6  0x00007fffea506a2a in mozilla::dom::Element::SetAttribute(nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) (this=this@entry=0x7fffca0dca60, aName=..., aValue=..., aError=...) at /var/tmp/firefox-esr-52.5.0esr/dom/base/Element.cpp:1246
#7  0x00007fffea927671 in mozilla::dom::ElementBinding::setAttribute(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) (cx=0x7fffe2413000, obj=..., self=0x7fffca0dca60, args=...) at /var/tmp/firefox-esr-52.5.0esr/build-browser/dom/bindings/ElementBinding.cpp:723
#8  0x00007fffeaa4248e in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (cx=<optimized out>, 
    cx@entry=0x7fffe2413000, argc=<optimized out>, vp=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/dom/bindings/BindingUtils.cpp:2904
#9  0x00007fffebe90f1e in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (args=..., native=<optimized out>, cx=0x7fffe2413000) at /var/tmp/firefox-esr-52.5.0esr/js/src/jscntxtinlines.h:239

this is however apparently not processed immediately, the Accessible* returned by GetAccessible() call in DocAccessible::AttributeChanged is stored in the event

- then the browser interface is actually updated, which apparently removes and reinserts some tabs, leading to an actual shutdown of the Accessible* referenced by the event, as per the following kind of backtrace:

#0  0x00007fffeb4cc5e6 in mozilla::a11y::Accessible::Shutdown() (this=0x7fffc82a2080)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/Accessible.cpp:1916
#1  0x00007fffeb4c51f9 in mozilla::a11y::DocAccessible::UnbindFromDocument(mozilla::a11y::Accessible*) (this=
    0x7fffc9a90350, aAccessible=0x7fffc82a2080)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/DocAccessible.cpp:1344
#2  0x00007fffeb4c89b1 in mozilla::a11y::DocAccessible::ShutdownChildrenInSubtree(mozilla::a11y::Accessible*) (this=<optimized out>, aAccessible=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/DocAccessible.cpp:2358
#3  0x00007fffeb4ae25f in mozilla::a11y::NotificationController::ProcessMutationEvents() (this=this@entry=0x7fffc8184460) at /var/tmp/firefox-esr-52.5.0esr/accessible/base/NotificationController.cpp:540
#4  0x00007fffeb4b09b9 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) (this=0x7fffc8184460, aTime=..., aTime@entry=...) at /var/tmp/firefox-esr-52.5.0esr/accessible/base/NotificationController.cpp:852
#5  0x00007fffeb4b03b3 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) (this=0x7fffc1061250, aTime=...) at /var/tmp/firefox-esr-52.5.0esr/accessible/base/NotificationController.cpp:638
#6  0x00007fffeb12a27e in nsRefreshDriver::Tick(long, mozilla::TimeStamp) (this=this@entry=0x7fffd7bf6000, aNowEpoch=aNowEpoch@entry=1515523355716437, aNowTime=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsRefreshDriver.cpp:1798

That was queued by this kind of backtrace:

#0  0x00007fffeb4acba6 in mozilla::a11y::NotificationController::ScheduleContentInsertion(mozilla::a11y::Accessible*, nsIContent*, nsIContent*) (this=0x7fffc80b7460, aContainer=0x7fffc8466080, aStartChildNode=<optimized out>, aEndChildNode=0x7fffc94b0940) at /var/tmp/firefox-esr-52.5.0esr/accessible/base/NotificationController.cpp:425
#1  0x00007fffeb4c4e2b in mozilla::a11y::DocAccessible::ContentInserted(nsIContent*, nsIContent*, nsIContent*) (this=<optimized out>, aContainerNode=aContainerNode@entry=0x7fffca859b80, aStartChildNode=aStartChildNode@entry=0x7fffc8064700, aEndChildNode=aEndChildNode@entry=0x7fffc94b0940)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/generic/DocAccessible.cpp:1365
#2  0x00007fffeb4b4c08 in nsAccessibilityService::ContentRangeInserted(nsIPresShell*, nsIContent*, nsIContent*, nsIContent*) (this=<optimized out>, aPresShell=<optimized out>, aContainer=aContainer@entry=0x7fffca859b80, aStartChild=aStartChild@entry=0x7fffc8064700, aEndChild=aEndChild@entry=0x7fffc94b0940)
    at /var/tmp/firefox-esr-52.5.0esr/accessible/base/nsAccessibilityService.cpp:566
#3  0x00007fffeb176da8 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) (this=this@entry=0x7fffce63f600, aContainer=0x7fffca859b80, aStartChild=aStartChild@entry=0x7fffc8064700, aEndChild=0x7fffc94b0940, aFrameState=<optimized out>, aAllowLazyConstruction=aAllowLazyConstruction@entry=false)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsCSSFrameConstructor.cpp:8225
#4  0x00007fffeb176eaa in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) (this=this@entry=0x7fffce63f600, aContainer=<optimized out>, aChild=aChild@entry=0x7fffc8064700, aFrameState=<optimized out>, aAllowLazyConstruction=aAllowLazyConstruction@entry=false)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsCSSFrameConstructor.cpp:7643
#5  0x00007fffeb177817 in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, bool, nsCSSFrameConstructor::RemoveFlags, nsIContent**) (this=this@entry=0x7fffce63f600, aContent=<optimized out>, 
    aContent@entry=0x7fffc8064700, aAsyncInsert=aAsyncInsert@entry=false, aFlags=aFlags@entry=nsCSSFrameConstructor::REMOVE_FOR_RECONSTRUCTION, aDestroyedFramesFor=aDestroyedFramesFor@entry=0x0)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsCSSFrameConstructor.cpp:9709
#6  0x00007fffeb177a7e in mozilla::RestyleManagerBase::ProcessRestyledFrames(nsStyleChangeList&) (this=0x7fffcf8fad60, aChangeList=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManagerBase.cpp:1177
#7  0x00007fffeb177f45 in mozilla::RestyleManager::ComputeAndProcessStyleChange(nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (this=this@entry=0x7fffcf8fad60, aFrame=aFrame@entry=
    0x7fffc8582558, aMinChange=aMinChange@entry=0, aRestyleTracker=..., aRestyleHint=aRestyleHint@entry=5, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:3804
#8  0x00007fffeb17862f in mozilla::RestyleManager::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (this=0x7fffcf8fad60, aElement=aElement@entry=0x7fffc8064700, aPrimaryFrame=aPrimaryFrame@entry=0x7fffc8582558, aMinHint=aMinHint@entry=0, aRestyleTracker=..., aRestyleHint=aRestyleHint@entry=5, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:153
#9  0x00007fffeb1787c7 in mozilla::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, mozilla::RestyleHintData const&) (this=this@entry=0x7fffcf8fadb8, aElement=0x7fffc8064700, aRestyleHint=5, aChangeHint=0, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleTracker.cpp:98
#10 0x00007fffeb17a05c in mozilla::RestyleTracker::DoProcessRestyles() (this=this@entry=0x7fffcf8fadb8)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleTracker.cpp:267
#11 0x00007fffeb17a58b in mozilla::RestyleManager::ProcessRestyles(mozilla::RestyleTracker&) (this=this@entry=0x7fffcf8fad60, aRestyleTracker=...) at /var/tmp/firefox-esr-52.5.0esr/build-browser/dist/include/mozilla/RestyleManager.h:490
#12 0x00007fffeb17a6de in mozilla::RestyleManager::ProcessPendingRestyles() (this=0x7fffcf8fad60)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:834
#13 0x00007fffeb1bd1a2 in mozilla::RestyleManagerHandle::Ptr::ProcessPendingRestyles() (this=<optimized out>)
    at /var/tmp/firefox-esr-52.5.0esr/build-browser/dist/include/mozilla/RestyleManagerHandleInlines.h:74
#14 0x00007fffeb1bd1a2 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) (this=0x7fffce637c00, aFlush=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsPresShell.cpp:4167
#15 0x00007fffeb1bd29e in PresShell::FlushPendingNotifications(mozFlushType) (this=<optimized out>, aType=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsPresShell.cpp:4054
#16 0x00007fffea56a0c2 in nsDocument::FlushPendingNotifications(mozFlushType) (this=0x7fffce65f000, aType=Flush_Layout)
    at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsDocument.cpp:7781
#17 0x00007fffea577a79 in nsFocusManager::CheckIfFocusable(nsIContent*, unsigned int) (this=this@entry=0x7fffe2423b30, aContent=aContent@entry=0x7fffc94af2c0, aFlags=aFlags@entry=2)
    at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsFocusManager.cpp:1557
#18 0x00007fffea595b3d in nsFocusManager::SetFocusInner(nsIContent*, int, bool, bool) (this=this@entry=0x7fffe2423b30, aNewContent=0x7fffc94af2c0, aFlags=aFlags@entry=2, aFocusChanged=aFocusChanged@entry=true, aAdjustWidget=aAdjustWidget@entry=true) at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsFocusManager.cpp:1181
#19 0x00007fffea5966fe in nsFocusManager::SetFocus(nsIDOMElement*, unsigned int) (this=0x7fffe2423b30, aElement=<optimized out>, aFlags=2) at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsFocusManager.cpp:484

i.e. the SetFocus call from javascript.

- eventually the attribute change event is processed, but since the Accessible* was shut down above, accessible/base/EventQueue.cpp's EventQueue::ProcessEventQueue see target->IsDefunct() and doesn't actually emit the StateChanged "selected" event.

Comment 2

[Samuel Thibault]

The issue is that currently Orca relies on these StateChanged "selected" events to announce the tabswitch.

It does also receive the addition/removal of the restyled tabs so in theory it could inspect them to check whether the newly-added tab has the "selected" attribute. I'm not sure Joanmarie will like to do that in Orca :)

Another possibility might be to introduce some synchronization point just after the JS this.setAttribute("selected") call, to make sure it gets propagated before SetFocus is called (which triggers the restyling and thus removal/insertion).

Comment 3

[Samuel Thibault]

Attachment: tabswitchacc

Oops, sorry, the Shutdown is not triggered as I said, but by the reconstruction queued by this:

#0  0x00007fffeb164fc2 in mozilla::RestyleManager::PostRestyleEvent(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, mozilla::RestyleHintData const*) (this=0x7fffcf8b1d60, aElement=0x7fffbfd75a60, aRestyleHint=eRestyle_Subtree, aMinChangeHint=0, aRestyleHintData=0x7fffffff8620)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:946
#1  0x00007fffeb1650fa in mozilla::RestyleManager::AttributeWillChange(mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (this=0x7fffcf8b1d60, aElement=aElement@entry=0x7fffbfd75a60, aNameSpaceID=aNameSpaceID@entry=0, aAttribute=aAttribute@entry=0x7fffdd7a5e60, aModType=aModType@entry=2, aNewValue=aNewValue@entry=0x0) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:286
#2  0x00007fffeb1866c6 in mozilla::RestyleManagerHandle::Ptr::AttributeWillChange(mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (aNewValue=0x0, aModType=2, aAttribute=0x7fffdd7a5e60, aNameSpaceID=0, aElement=0x7fffbfd75a60, this=<optimized out>)
    at /var/tmp/firefox-esr-52.5.0esr/build-browser/dist/include/mozilla/RestyleManagerHandleInlines.h:139
#3  0x00007fffeb1866c6 in PresShell::AttributeWillChange(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (this=0x7fffcef57400, aDocument=<optimized out>, aElement=0x7fffbfd75a60, aNameSpaceID=0, aAttribute=0x7fffdd7a5e60, aModType=2, aNewValue=0x0)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsPresShell.cpp:4328
#4  0x00007fffea5bc268 in nsNodeUtils::AttributeWillChange(mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) (aElement=aElement@entry=0x7fffbfd75a60, aNameSpaceID=aNameSpaceID@entry=0, aAttribute=aAttribute@entry=0x7fffdd7a5e60, aModType=2, aNewValue=aNewValue@entry=0x0)
    at /var/tmp/firefox-esr-52.5.0esr/dom/base/nsNodeUtils.cpp:132
#5  0x00007fffea51955a in mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) (this=0x7fffbfd75a60, aNamespaceID=0, aName=0x7fffdd7a5e60, aPrefix=0x0, aValue=..., aNotify=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/dom/base/Element.cpp:2367
#6  0x00007fffea506a2a in mozilla::dom::Element::SetAttribute(nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) (this=this@entry=0x7fffbfd75a60, aName=..., aValue=..., aError=...) at /var/tmp/firefox-esr-52.5.0esr/dom/base/Element.cpp:1246
#7  0x00007fffea927671 in mozilla::dom::ElementBinding::setAttribute(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) (cx=0x7fffe2413000, obj=..., self=0x7fffbfd75a60, args=...) at /var/tmp/firefox-esr-52.5.0esr/build-browser/dom/bindings/ElementBinding.cpp:723
#8  0x00007fffeaa4248e in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (cx=<optimized out>, 
    cx@entry=0x7fffe2413000, argc=<optimized out>, vp=<optimized out>) at /var/tmp/firefox-esr-52.5.0esr/dom/bindings/BindingUtils.cpp:2904

Where the setAttribute call is the same that triggered the "selected" event, so no javascript barrier can fix that :)

So what happens is that both the "selected" notification event and the restyle event get posted from the same SetAttribute JS call, and it just happens that WillRefresh processes the latter before the former.  I would argue that WillRefresh should process the pending notification events before processing the content insertion events, as the attached patch does.

Joanmarie, would you be able to test it to confirm that things are working fine with Orca?

Comment 4

[alexander :surkov (:asurkov)]

Comment on attachment 8942663 [details] [diff] [review]
tabswitchacc

Review of attachment 8942663 [details] [diff] [review]:
-----------------------------------------------------------------

It doesn't appear a correct approach with me, because if something is about to destroy then it doesn't make sense to fire events for that thing. This case is not different than adding an element into DOM with all attributes set, for example, container.innerHTML = `<div role='tab' aria-selected>tab</div>`, where only show event is fired. Shouldn't it be Orca responsibility to handle this case?

Having said that, I'd say it probably doesn't make sense to recreate an accessible in this particular case, which could be also a fix on Gecko's side.

Comment 5

[Samuel Thibault]

I seems that the recreation is requested from restyling rules: the stack below seems to be what adds  nsChangeHint_ReconstructFrame to the ChangeList:

#0  0x00007fffeb147d99 in mozilla::ElementRestyler::MaybeReframeForPseudo(mozilla::CSSPseudoElementType, nsIFrame*, nsIFrame*, nsIContent*, nsStyleContext*) (this=0x7fffffffaf70, aPseudoType=<optimized out>, aGenConParentFrame=<optimized out>, aFrame=0x7fffbf51b720, aContent=0x7fffc8114280, aStyleContext=<optimized out>)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:3571
#1  0x00007fffeb16756e in mozilla::ElementRestyler::RestyleChildren(nsRestyleHint) (this=0x7fffffffaf70, aChildRestyleHint=eRestyle_Subtree)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:3229
#2  0x00007fffeb166330 in mozilla::ElementRestyler::Restyle(nsRestyleHint) (this=this@entry=0x7fffffffaf70, aRestyleHint=aRestyleHint@entry=eRestyle_Subtree)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:2291
#3  0x00007fffeb16705e in mozilla::ElementRestyler::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&, nsTArray<mozilla::ElementRestyler::ContextToClear>&, nsTArray<RefPtr<nsStyleContext> >&) (aFrame=aFrame@entry=0x7fffbf51b720, aChangeList=aChangeList@entry=0x7fffffffb2e8, aMinChange=aMinChange@entry=0, aRestyleTracker=..., aRestyleHint=aRestyleHint@entry=eRestyle_Subtree, aRestyleHintData=..., aContextsToClear=..., aSwappedStructOwners=...)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:3390
#4  0x00007fffeb176f60 in mozilla::RestyleManager::ComputeAndProcessStyleChange(nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (this=this@entry=0x7fffcea8eae0, aFrame=aFrame@entry=0x7fffbf51b720, aMinChange=aMinChange@entry=0, aRestyleTracker=..., aRestyleHint=aRestyleHint@entry=eRestyle_Subtree, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:3803
#5  0x00007fffeb177655 in mozilla::RestyleManager::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (this=0x7fffcea8eae0, aElement=aElement@entry=0x7fffc8114280, aPrimaryFrame=aPrimaryFrame@entry=0x7fffbf51b720, aMinHint=aMinHint@entry=0, aRestyleTracker=..., aRestyleHint=aRestyleHint@entry=eRestyle_Subtree, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:153
#6  0x00007fffeb1777ed in mozilla::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, mozilla::RestyleHintData const&) (this=this@entry=0x7fffcea8eb38, aElement=0x7fffc8114280, aRestyleHint=eRestyle_Subtree, aChangeHint=0, aRestyleHintData=...) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleTracker.cpp:98
#7  0x00007fffeb179082 in mozilla::RestyleTracker::DoProcessRestyles() (this=this@entry=0x7fffcea8eb38) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleTracker.cpp:267
#8  0x00007fffeb1795b1 in mozilla::RestyleManager::ProcessRestyles(mozilla::RestyleTracker&) (this=this@entry=0x7fffcea8eae0, aRestyleTracker=...)
    at /var/tmp/firefox-esr-52.5.0esr/build-browser/dist/include/mozilla/RestyleManager.h:490
#9  0x00007fffeb179704 in mozilla::RestyleManager::ProcessPendingRestyles() (this=0x7fffcea8eae0) at /var/tmp/firefox-esr-52.5.0esr/layout/base/RestyleManager.cpp:834
#10 0x00007fffeb1bc1c8 in mozilla::RestyleManagerHandle::Ptr::ProcessPendingRestyles() (this=<optimized out>)
    at /var/tmp/firefox-esr-52.5.0esr/build-browser/dist/include/mozilla/RestyleManagerHandleInlines.h:74
#11 0x00007fffeb1bc1c8 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) (this=0x7fffce277800, aFlush=...)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsPresShell.cpp:4167
#12 0x00007fffeb12941b in nsRefreshDriver::Tick(long, mozilla::TimeStamp) (this=this@entry=0x7fffce277000, aNowEpoch=aNowEpoch@entry=1516292171623172, aNowTime=...)
    at /var/tmp/firefox-esr-52.5.0esr/layout/base/nsRefreshDriver.cpp:1836


It seems to me that it's too deep in the code for a fix.

Comment 6

[Samuel Thibault]

Attachment: orca patch to announce selected tab insertion

This is a crude attempt at adding the support in Orca to announce the insertion of selected tabs. In my tests it seems to be going fine.

Comment 7

[Joanmarie Diggs]

(In reply to alexander :surkov from comment #4)

> Having said that, I'd say it probably doesn't make sense to recreate an
> accessible in this particular case, which could be also a fix on Gecko's
> side.

Agreed and preferred. Orca already has to process a lot of children-changed events. If the GUI element in question continues to exist, destroying and recreating it is not helpful.

Comment 8

[Joanmarie Diggs]

(In reply to Samuel Thibault from comment #6)
> Created attachment 8943699 [details] [diff] [review]
> orca patch to announce selected tab insertion
> 
> This is a crude attempt at adding the support in Orca to announce the
> insertion of selected tabs. In my tests it seems to be going fine.

Without using your crude attempt at all, and using Orca master with Firefox Nightly, when I press Control+Page Up/Page Down my experience is that Orca almost always presents the page tab right away. The rest of the time, it also presents the page tab but after presenting other page information (e.g. the document URL). So what problem are you trying to fix? The ordering on the few occasions it's after other page information or something else?

Comment 9

[Samuel Thibault]

Well, see my report: without the patch, I'm mostly only getting tab title reports when going forward. That was with firefox 52 and 57 though, not nightly. Also, this is without e10s.

Comment 10

[Joanmarie Diggs]

(In reply to Samuel Thibault from comment #9)
> Well, see my report: without the patch, I'm mostly only getting tab title
> reports when going forward. That was with firefox 52 and 57 though, not
> nightly. Also, this is without e10s.

Is it fixed for you in Nightly?

Comment 11

[Samuel Thibault]

Oh, it seems it work fine with nightly without e10s too indeed.

Comment 12

[Joanmarie Diggs]

(In reply to Samuel Thibault from comment #11)
> Oh, it seems it work fine with nightly without e10s too indeed.

So is there anything to do with this bug here? I don't plan on modifying Orca to support something that will JustWork(tm) once the changes in Nightly wind up in a stable version of Firefox.

Comment 13

[Samuel Thibault]

Ok, we don't find cases where nightly still has the issue, so I'd say we can consider this bug fixed for 59.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to Samuel Thibault from comment #13)
> Ok, we don't find cases where nightly still has the issue, so I'd say we can
> consider this bug fixed for 59.

Comment 7 approach is a long way to have a chance to get in 59, here's a tracking bug 1285862. You may want to call it wontfix. 

(In reply to Joanmarie Diggs from comment #12)
> (In reply to Samuel Thibault from comment #11)
> > Oh, it seems it work fine with nightly without e10s too indeed.
> 
> So is there anything to do with this bug here? I don't plan on modifying
> Orca to support something that will JustWork(tm) once the changes in Nightly
> wind up in a stable version of Firefox.

Joanie, it is probably wider issue than Firefox UI issue. How would you handle the case:

el.innerHTML = `<div role='tab' aria-selected='true' tabindex='0'>Tab1</div>`;
el.firstChild.focus();

Is the scenario different from this bug?

Comment 15

[Joanmarie Diggs]

This bug is, I think, a duplicate of 1260598.

Comment 16

[alexander :surkov (:asurkov)]

(In reply to Joanmarie Diggs from comment #15)
> This bug is, I think, a duplicate of 1260598.

it shouldn't be, since bug 1260598 was fixed in 59.

Comment 17

[Joanmarie Diggs]

In comment #13 Samuel says:

> so I'd say we can consider this bug fixed for 59.

If that means he's not seeing the problem in 59, then.... Besides, it's a dup of *something*. :) It's fixed in (at least) Nightly. Orca master is doing the right thing without anything else needing to be added to Orca master. Dunno what else to tell you....

Comment 18

[alexander :surkov (:asurkov)]

(In reply to Joanmarie Diggs from comment #17)
> In comment #13 Samuel says:
> 
> > so I'd say we can consider this bug fixed for 59.
> 
> If that means he's not seeing the problem in 59, then.... Besides, it's a
> dup of *something*. :) It's fixed in (at least) Nightly. Orca master is
> doing the right thing without anything else needing to be added to Orca
> master. Dunno what else to tell you....

sorry, I misread Samuel's comment.

Anyway, bug 1260598 might be not a dupe of this one, since it was related to queuing events in the main and content processes properly, but I leave this to Samuel to make a call. I'm glad to hear this bug is fixed.

Comment 19

[Samuel Thibault]

Yes, I don't think bug 1260598 is a duplicate of this, since with firefox 52 or 57 plus the patch fixing them, I'm getting events in the proper order, I'm just not getting the "selected" statechanged event, only frame reconstruction.

With the current nightly I'm always getting the statechanged events, and in the proper order, except Bug 1432432, but that case is not very annoying.

So I believe for the long run it is fixed.

Comment 20

[alexander :surkov (:asurkov)]

so worksforme based on comment #19