When an OBJECT_SHOW event is fired, its MSAA states are not properly set yet.

RESOLVED FIXED

Status

()

P1
major
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: MarcoZ, Assigned: davidb)

Tracking

(Blocks: 1 bug, {access})

Trunk
access
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 9 obsolete attachments)

(Reporter)

Description

11 years ago
Jamie and I found this while trying to eliminate the speaking of live region text in background tabs of ChatZilla output windows. I tried to test for STATE_SYSTEM_OFFSCREEN (MSAA), which I found present on a hidden tab's table cells properties. However, this state is not present yet when the object is first added (when a new message comes in) and its "show" event is fired.

STR:
1. Start Firefox and ChatZilla.
2. Visit a busy channel and one not so busy. Make sure the busy channel is in the background.
3. Start AccEvent32. Set it to only show OBJECT_SHOW and OBJECT_STATECHANGED events and leave the event info and object data as is.
4. Clear the event output, then focus back to the ChatZilla window and wait a while until a few messages have come in on both channels.
5. Go back to AccEvent32.

Result: A lot of EVENT_SHOW events, one for each table cell that contains either a nickname or an output. All of them have state "normal".
No state change events at all on table cells.

6. Now start AccProbe and navigate to your ChatZilla output, and drill down to the nodes of a foreground and a background tab. The cells on the background tab all have the STATE_SYSTEM_:OFFSCREEN set, while the foreground ones do not, which is correct.

Expected: When the events fire for the cells in the background tabs, the STATE_SYSTEM_OFFSCREEN should already be present.

Interesting side note: All IA2 states were already present when the OBJECT_SHOW event was fired. Checked that with NVDA by asking for MSAA states and IA2 states separately. While the MSAA states were always 0, the IA2 states were non-zero, reflecting what would finally be visible in AccProbe as well.
Flags: wanted1.9.1?
(Reporter)

Comment 1

11 years ago
This hinders NVDA from properly implementing live region support. Live Regions are an integral part of ARIA. Requesting blocking and setting priority.
Flags: blocking1.9.1?
Priority: -- → P1
(In reply to comment #0)

is it possible to create separate testcase?

> Interesting side note: All IA2 states were already present when the OBJECT_SHOW
> event was fired. Checked that with NVDA by asking for MSAA states and IA2
> states separately. While the MSAA states were always 0, the IA2 states were
> non-zero, reflecting what would finally be visible in AccProbe as well.
> 

that's quite strange

Updated

11 years ago
Blocks: 343213
Marco, is this a regression from Fx 3?
(Reporter)

Comment 4

11 years ago
(In reply to comment #3)
> Marco, is this a regression from Fx 3?

No, this problem is also present in FX 3.0.x.

Updated

10 years ago
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
There has been some work that may have impacted this. Does it still present?
(Reporter)

Comment 6

10 years ago
This is still present in the combination  (Quit: ChatZilla 0.9.84 [Firefox 3.2a1pre/20090228034747].
Assignee: nobody → david.bolter
(Reporter)

Comment 7

10 years ago
I just tested this with ChatZilla and Orca on Linux, and it's reproducible there, too. Text from background tabs leaks through and is spoken via live region updates, like on Windows.
OS: Windows XP → All
Hardware: x86 → All
Hardware: All → x86
Hardware: x86 → All
I've reproduced on Mac via trace output.

Do we want to send these events at all for background tabs?
Please disregard comment #8.

Debugging in our core at nsAccessible::FireAccessibleEvent
When a table row (chat message) is inserted, we have three events:
1. EVENT_TEXT_INSERTED
2. EVENT_DOM_CREATE
3. EVENT_REORDER

In all three cases the related accessible has nsIAccessibleStates::STATE_OFFSCREEN set (aside: these accessibles have no accessible name).

For windows the follow mappings should happen:
1. EVENT_TEXT_INSERTED (should be mapped to IA2_EVENT_TEXT_INSERTED)
2. EVENT_DOM_CREATE (should be mapped to MSAA EVENT_OBJECT_SHOW)
3. EVENT_REORDER (should be mapped to MSAA EVENT_OBJECT_REORDER)

I wonder if someone could please go back to AccEvent32 and attach a log of the event information when a chat message appears in a background tab?
Uhm.

I'm looking at nsAccessibleWrap::get_states (in accessible/src/msaa). We call:
GetState(&states, &extraStates)

But don't seem to use "states" sufficiently to properly fill the outparam "aStates".

I must be missing something?
Posted patch exploratory fix (obsolete) — Splinter Review
Marco, can you try this (untested, uncompiled) patch against NVDA with your modified event_show which includes a check for controlTypes.STATE_OFFSCREEN?

If this works you can keep the change in NVDA, but I'd like to make the FF fix more general/powerful.
Iirc get_states of IA2 exposes IA2 states only, AT use get_accstates of IA to get MSAA states.
Posted patch exploratory (obsolete) — Splinter Review
(In reply to comment #12)
> Iirc get_states of IA2 exposes IA2 states only, AT use get_accstates of IA to
> get MSAA states.

Thanks Alexander. This would be the right exploratory patch then.

I really need to get my windows dev vm set up.
Attachment #371718 - Attachment is obsolete: true
Posted patch WIP 1 (obsolete) — Splinter Review
Marco, can you try this patch out?
Attachment #372446 - Attachment is obsolete: true
Posted patch WIP 2 (obsolete) — Splinter Review
Actually this should be more robust. Calling Flush_Layout should update the rect visibility which we need for state offscreen. (thx roc and bz)
Attachment #375289 - Attachment is obsolete: true
Posted patch WIP 3 (obsolete) — Splinter Review
removed bailing when no dom node, since we later want to check and expose state defunct in the extra state outparam.
Attachment #375292 - Attachment is obsolete: true
(Reporter)

Comment 17

10 years ago
The results with this latest patch are inconsistent. First of all, I needed to apply it to mozilla-1.9.1, not mozilla-central, because mozilla-central currently has a broken accessible tree for role="log", at least with NVDA.

I have a feeling the situation is a bit better, in that not everything leaks through from all ChatZilla tabs. Some things, however, do still leak through, and I haven't yet found out what exactly triggers it. One thing that always comes through is an assertive message, for example something with my nick in it will be spoken in both the current channel and the server info tab. Other stuff such as Firebot messages don't get leaked from other channels most of the time, but again, not always consistently suppressed.

So this general direction seems to be right, we're njust not completely there yet.
Attachment #375332 - Attachment is patch: true
Attachment #375332 - Attachment mime type: application/octet-stream → text/plain
Posted patch WIP 4 (obsolete) — Splinter Review
Marco, using Accessibility Probe, these events are appearing invisible and offscreen for each background chat message:
EVENT_OBJECT_REORDER
IA2_EVENT_TEXT_INSERTED
EVENT_OBJECT_SHOW

Can you give it a whirl with NVDA on 1.9.1?
Attachment #375332 - Attachment is obsolete: true
(Note I still need to confirm the MSAA state problem)
Status: NEW → ASSIGNED
(Reporter)

Comment 20

10 years ago
Here are my findings with the latest patch:
If I let NVDA create a particular channel's buffer, that channel will leak through. If I just open a channel and let it chatter away without switching to it with F6, as soon as I switch to a different channel, the previous channel won't leak through.

This is a difference from behaviour without the patch, where channels would *always* leak through whether I had NVDA create a buffer for it or not.

So the following sequence of steps can repro the bug:
1. Open Shiretoko compiled with this patch.
2. Open ChatZilla.
3. Without switching to the start pane, with the focus simply in the edit field, type /sslserver irc.mozilla.org 6697 and hit enter.
4. Once that server finishes loading, type /join #accessibility.
5. Listen to stuff as it comes in.
6. Type /join #maildev (or any other channel of your preference) where you know that has regular updates.
7. Let it chatter away. Notice that nothing from the accessibility channel is spoken.
8. Press Ctrl+Shift+Tab to switch back to #accessibility. Notice that now the #accessibility stuff gets spoken, but the other channel is silent.
9. Press F6. This will put focus in the output window, which is HTML content, and will make NVDA generate the virtual buffer.
10. Press Ctrl+Tab. This will switch to the other channel and put focus back to the edit field. Notice that now, chatter comes from both the #maildev and #accessibility channels.
11. Switch back to #accessibility. Notice that now, only #accessibility updates will be spoken. Reason: #maildev still has no virtual buffer created.

These steps repro stuff consistently for me, and again, this is different than without the patch.
That is some awesome research. Jamie, Mick, can you dig into this on your side? I'm wondering why the virtual buffer would get in the way here.
(Reporter)

Comment 22

10 years ago
Mick or Jamie, do you have any idea why NVDA might get state bits differently once it creates a virtual buffer for a tab? See my previous comment with the steps to repro. Let me know if you need a build of Firefox with this patch included.
(In reply to comment #22)
> Mick or Jamie, do you have any idea why NVDA might get state bits differently
> once it creates a virtual buffer for a tab?
This is very odd. The only thing I can think of is that we hold a reference to the root document object once a buffer is created and we don't release that reference until the buffer is destroyed. I don't understand why this would affect the states on new objects, though. If I'm correct, you should see this problem in AccProbe as well if you first let NVDA create a buffer.

This theory should be relatively easy to test:
1. Do *not* let NVDA create a buffer, but instead, find the output window with the object navigator.
2. Turn off "Caret moves review cursor" (NVDA+6) and "Focus moves navigator object" (NVDA+7) to stop the navigator from moving.
3. Now ctrl+tab to the other channel and check for leakage.
(Reporter)

Comment 24

10 years ago
OK, I did the test, and here's what I found:
1. The rule of "virtual buffer causing the tabs to start leaking" is more general. Basically, it is enough if I let NVDA create a virtual buffer for *any* channel, as soon as I do that, the background tabs start leaking through. I confirmed this twice.
2. As long as I don't create a virtual buffer anywhere within my ChatZilla session, I can use the Object Navigator to walk into the current output window, leave it there, and shift focus to a different tab. Only the live region updates from that tab will then be spoken, but no background tabs.
These, again, start leaking as soon as I create a v buffer for the current tab.
Is it possible NVDA is not updating the offscreen state if/when storing objects in the virtual buffer? Jamie? This is a pickle.
Posted patch WIP 4.1 (check IsDefunct) (obsolete) — Splinter Review
Attachment #375703 - Attachment is obsolete: true
The generation of the virtual buffer and NVDA's access to the objects for other purposes are completely separate calls using different instances of the object. To make the separation even clearer, the virtual buffer is generated in-process, while the rest of NVDA is out-of-process. The state bits cannot be shared between the two for the purposes of live regions, as live regions are not handled by the virtual buffer.

It is worth noting that both NVDA and the virtual buffer will handle events for objects within the buffer, the former in-process, the latter out-of-process. My theory is that once the object is instantiated once (or perhaps the first time it is queried for states), subsequent instantiation/queries are incorrect. One way to test this would be to run two instances of accProbe. To really duplicate the test, you probably want to run one in-process and one out-of-process.

Failing this, Marco, as offered earlier, can you please provide a build which includes this patch?
(In reply to comment #27)
> Failing this, Marco, as offered earlier, can you please provide a build which
> includes this patch?

I went ahead and started a build. Should be here soon: https://build.mozilla.org/tryserver-builds/2009-05-07_18:20-dbolter@mozilla.com-bug444644/
(In reply to comment #27)
> Failing this, Marco, as offered earlier, can you please provide a build which
> includes this patch?

Hi James, did you get a chance to try the patched build?
My testing reveals that this is not related to NVDA's virtual buffers. I performed the following steps with the buffers disabled. This should also be demonstrable using AccProbe or the like.

Marco, you can disable the buffers by changing this line in virtualBufferHandler.py:
classString="virtualBuffers.gecko_ia2.Gecko_ia2"
to:
return

Steps:
1. Open Chatzilla and join a channel (channel1).
2. Join a second channel (channel2).
3. Press f6 to move to the output window.
4. Press shift+ctrl+tab to move to channel1.
5. Observe that messages from the background tab (channel2) do not have the offscreen or invisible states.

6. Press ctrl+tab to move to channel2.
7. Press f6 to move to the output window.
8. Press shift+f6 to move back to the input window.
9. Press shift+ctrl+tab to move to channel1.
10. Observe that messages from the background tab (channel2) *do* have the offscreen and invisible states.

Overall observation: If focus is in the output window when moving to a different channel, the offscreen and invisible states are not set correctly. If focus is not in the output window, all is well.
(In reply to comment #30)
> Overall observation: If focus is in the output window when moving to a
> different channel, the offscreen and invisible states are not set correctly. If
> focus is not in the output window, all is well.

Thanks for documenting your findings Jamie, and thanks for working on channel tonight. Perhaps we are doing something funky in gecko based on a document's gLastFocusedNode. I'll need to get familiar with that code.

Comment 32

10 years ago
So basically, we have an inputbox key press handler: http://mxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/handlers.js#336
which seems to just throw such an event away.

Then we have a window one: http://mxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/handlers.js#539 which seems to do the right thing.

It would be good to check with a debugger (probably not breaking but counting the number of hits on a breakpoint) if the latter of those fires for both cases.

I mean, in theory if you do this in the input box, it'll hit the event handler for the box, not do anything, then hit the one on the window, and then Just Work. The one on the chat window should just hit the window one.

I'm not sure where that logic fails, then, or how internal state gets subtly altered by what happens so that it ends up with a11y breaking.

Based on the fact that it fails for the output window, I would be tempted to say it might be the tab handlers itself taking care of the tab switch, which might not change internal state in ChatZilla as to what the current content window is.

The tabpanel/tabs stuff we have is kind of... funky... because of old issues with the "normal" implementation. It uses a deck, and manually decides which thing is visible for a given selected tab. I would test some of the above myself if I wasn't on mac - is there a way I could still test for the accessibility states you mention on that platform?
Will be interesting to re-test this bug after bug 178324 is fixed.
Marco, I pushed the log role fix so testing this should be easier. I think chatzilla tapanel/tabs stuff "is kind of... funky..." might indeed be hitting us here after all (comment #32). There are a few more tests I want to try before punting this over.

Gijs, can you tell us what aria live stuff chatzilla is currently using? Including details on when polite and assertive would be great, and also note that aria-channel is not supported.
(Reporter)

Comment 35

10 years ago
I'm testing this with a custom built ChatZilla that 's already updated with the aria-channel removed. ChatZilla uses role="log" on the table that holds the output, and uses aria-live="assertive" whenever it needs to ping a user (same as when the icon in the status bar starts flashing).

Comment 36

10 years ago
In addition to what Marco said (which is accurate), https://wiki.mozilla.org/User:GijsKruitbosch/ChatA11Y_AppAuthors#Example:_ChatZilla might help.
Posted patch patch 1 (obsolete) — Splinter Review
This patch is reported by Marco to improve behavior. There is still work to be done investigating the chatzilla focus weirdness (see comment #30).

The consensus is to land this patch, and file a separate bug for the chatzilla focus investigation, pointing back to this bug.
Attachment #376298 - Attachment is obsolete: true
Attachment #383491 - Flags: review?(surkov.alexander)
Attachment #383491 - Flags: review?(marco.zehe)
(Reporter)

Updated

10 years ago
Attachment #383491 - Flags: review?(marco.zehe) → review+
Attachment #383491 - Flags: review?(surkov.alexander)
Comment on attachment 383491 [details] [diff] [review]
patch 1


>+  if (!IsDefunct()) {

Why you don't use early return?

>+    // update view (and styles etc)
>+    nsCOMPtr<nsIPresShell> presShell = nsCoreUtils::GetPresShellFor(mDOMNode);

it's more robust to use nsAccessNode::GetPresShell() I think

>+    presShell->FlushPendingNotifications(Flush_Layout);

every call should be very good documented, i.e. why it is necessary here.
btw, could your approach help to the bug 498015?
(In reply to comment #38)
> (From update of attachment 383491 [details] [diff] [review])
> 
> >+  if (!IsDefunct()) {
> 
> Why you don't use early return?

I can't bail here because we want to expose the EXT_STATE_DEFUNCT (in GetStateInternal) :)


> 
> >+    // update view (and styles etc)
> >+    nsCOMPtr<nsIPresShell> presShell = nsCoreUtils::GetPresShellFor(mDOMNode);
> 
> it's more robust to use nsAccessNode::GetPresShell() I think

Done.

> 
> >+    presShell->FlushPendingNotifications(Flush_Layout);
> 
> every call should be very good documented, i.e. why it is necessary here.

Will do.
(In reply to comment #39)
> btw, could your approach help to the bug 498015?

I've asked Rob to take a look.
Posted patch patch 2 (obsolete) — Splinter Review
Addresses comments.
Attachment #383491 - Attachment is obsolete: true
Attachment #383762 - Flags: review?(surkov.alexander)
Attachment #383762 - Flags: review?(surkov.alexander)
Comment on attachment 383762 [details] [diff] [review]
patch 2


>+    // Flush layout so that all the frame construction, reflow, and styles are
>+    // up-to-date. We don't flush the display because we don't care about
>+    // painting.

It's much better :) But I'd like to see comment why we need to flush layout in THIS place exactly.

I understand we should flush layout when calculating the states because we rely on layout to grab the states (but we need to document this). What about events?

waiting for comments and new patch :)
Attachment #383854 - Flags: review?(surkov.alexander)
Comment on attachment 383854 [details] [diff] [review]
patch 3 - improved comments

r=me
Attachment #383854 - Flags: review?(surkov.alexander) → review+
Thanks. I filed bug 499068 for follow up.

Now what remains for this bug is filing a Chatzilla bug with whatever information we have about how its custom algorithms for deciding what tabs are visible might be failing. (see comment #32).

Gijs, did you get your windows env set up?

Comment 47

10 years ago
(In reply to comment #46)
> Thanks. I filed bug 499068 for follow up.
> 
> Now what remains for this bug is filing a Chatzilla bug with whatever
> information we have about how its custom algorithms for deciding what tabs are
> visible might be failing. (see comment #32).
> 
> Gijs, did you get your windows env set up?

No. I realised that my CD/DVD drive actually doesn't work - something which I've been meaning to go to an Apple Store about, but haven't so far, and right now I can't seem to get a Genius bar reservation for the next 3 days... Blegh. Anyway, I can try at uni, but I won't be able to build there (no disk space).


Actually, now that I think about it... why is this a ChatZilla problem? From comment #30, wouldn't it be Gecko's job to take focus away from an element that's disappearing, and pick the Next Best Thing or the main window or something? I'd guess that's all that's really wrong...
(In reply to comment #47)
> Actually, now that I think about it... why is this a ChatZilla problem? From
> comment #30, wouldn't it be Gecko's job to take focus away from an element
> that's disappearing, and pick the Next Best Thing or the main window or
> something? I'd guess that's all that's really wrong...

As per IRC:
We can't recreate this problem in FF proper and we have code to ensure something is always focused (which presumably removes focus from the previously focused element automatically). I might be able to give more detailed diagnosis but I was having general almost-hang issues with the latest CZilla and tools like Accprobe on Windows.
Attaching a test (originally from Gijs) used to confirm this bug is not reproducible in firefox+gecko. To use the test, unzip the files and load accessibility.html. Open a new tab and confirm that new messages in the background tab don't leak through (have state OFFSCREEN).
Pushed as changeset: http://hg.mozilla.org/mozilla-central/rev/456d6d0bd8a4

Opened bug 499731 for pixie hunting.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Follow up: that changeset combined with our frame tree walker and the frame poisoning work are thought to have caused bug 525579. We might back this out (at least) for 1.9.2.
You need to log in before you can comment on or make changes to this bug.