Closed Bug 660083 Opened 14 years ago Closed 12 years ago

missing focus event when opening a message in Thunderbird

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Version:
10 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla20

People

(Reporter: mgorse, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

log
122.80 KB, text/plain
Details
patch4
12.58 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 Build Identifier: Thunderbird 3.4a1pre When pressing ctrl-w from the inbox in Thunderbird 3.3 or 3.4, a focus event is not fired, where a focus event is fired for 3.1. Orca thus does not recognize that the message now has focus, so it does not read it, and cursoring does not cause the current line to be read. (I wonder if this is related to the fix for bug 615220, but that is just a guess on my part.) Reproducible: Always Steps to Reproduce: 1. Open Accerciser or something similar, and monitor for events. 2. Press ctrl-o to open a message in Thunderbird. 3. Actual Results: Several events are fired, including a document:load-complete, but there is no focus event. Expected Results: I would expect that a focus event would be fired, as in 3.1.
Blocks: focuseventa11y
No longer blocks: eventa11y
Fernando, yours one?
Could be. Mike, could you please try this build on thunderbird: http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/fherrera@mozilla.com-cc5c2d87f7f5/ It includes the patch on bug #639835
Sorry for the delay in responding. Anyway, I just tried the build that you linked to, and it doesn't change the behavior that I mentioned--I still don't see a focus event when pressing ctrl-o to open a message.
Confirmed in Windows. This is a common complaint from NVDA users and we don't recommend that users upgrade past Thunderbird 3.1 due to this bug. The easiest way to reproduce: 1. Set Thunderbird to open messages in a new tab. 2. Move to a message in the message list. 3. Press enter. Some users have reported being able to reproduce this even when Thunderbird is set to open messages in a new window, but most (including Mick and me) can't reproduce this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
please check if this bug is still present after bug 673958 is fixed
Status: NEW → UNCONFIRMED
Ever confirmed: false
Target Milestone: --- → mozilla7
Version: unspecified → 10 Branch
Target Milestone: mozilla7 → ---
(In reply to alexander surkov from comment #6) > please check if this bug is still present after bug 673958 is fixed Afraid so. Interestingly, the message document does get the focused state, but no focus event is fired on it.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file log —
Due to some reason thunderbird creates two documents for the open message: 1) create a document for the message 2) focus it 3) destroy the document for the message 4) create new document for the message That's likely Gecko bug because new document doesn't get focus event. But I need a help from someone from Thunderbird team to know what special they do here.
Alexander, is this problem specific to opening a message in a new tab? Or is it general to switching messages? I'm not at all familiar with the tab code, though I can probably find the code for you...as to why it's not generating a focus event, I have no idea.
(In reply to David :Bienvenu from comment #9) > is this problem specific to opening a message in a new tab? Or is > it general to switching messages? It's general to switching messages.
(In reply to David :Bienvenu from comment #9) > Alexander, is this problem specific to opening a message in a new tab? I tried that when I commented my observations in comment #8. I need a help to figure out why Thunderbird creates two DOM documents for the message. (In reply to James Teh [:Jamie] from comment #10) > (In reply to David :Bienvenu from comment #9) > > is this problem specific to opening a message in a new tab? Or is > > it general to switching messages? > It's general to switching messages. this might be two different bugs, I'll take a look what happens in accessibility when switching between messages.
To clarify, i was referring to switching between messages in two different tabs; e.g. pressing control+tab. I'm not sure about switching messages in the same tab.
about switching the between messages, I can see focus event coming after document load complete event. It seems due to some reason Thunderbird reloads message but doing that with NVDA running I can hear it reads the document content well. Jamie, what version of Thunderbird do you try?
(In reply to alexander surkov from comment #13) > about switching the between messages, I can see focus event coming after > document load complete event. It seems due to some reason Thunderbird > reloads message but doing that with NVDA running I can hear it reads the > document content well. Jamie, what version of Thunderbird do you try? I see that happens when I switch to any message tab from tab containing all messages - no focus event at all.
we re-use docshells when switching messages, but if you're coming from a tab that's not displaying a document, it might be that we just switch decks. Asuth might have some insights...
Yeah, we don't force a focus event when we change tabs. There was some bug I commented on recently to this effect where amusing things can happen if you switch between a message tab and a content tab, and then type stuff, you can end up acting on the message you were just looking at. Anything that seemed to work correctly probably would just be a side-effect of us re-triggering message display on tab switch where the message pane is visible in the target.
Er, let me restate that. The multiplexed mail tab type does do some magic about making sure the right thing has focus when we change to that tab, but we will not synthesize a focus event if the right thing is already focused. That other bug was that we leave it up to tabs to take care of focus on their own, which is a problem once we are actually changing what is displayed in the deck, like when content pages get involved.
Thx, Andrew. So, *should* we be synthesizing a focus event in this case, as a notification? If so, which code should Alexander look at?
(In reply to alexander surkov from comment #13) > about switching the between messages, I can see focus event coming after > document load complete event. It seems due to some reason Thunderbird > reloads message but doing that with NVDA running I can hear it reads the > document content well. Sorry, my bad. It works just fine when switching between message tabs. As you later pointed out, it doesn't work when switching between a message list tab and a message tab, which is what I must have been testing.
(In reply to David :Bienvenu from comment #18) > Thx, Andrew. So, *should* we be synthesizing a focus event in this case, as > a notification? If so, which code should Alexander look at? So, (regardless of this specific bug) Thunderbird needs to perform some kind of focusing in tabmail.xml. It is not doing so, but mailTabs.js is doing its own focus maintenance (and that is likely what would have changed between Thunderbird 3.1 and later versions). Given that screen-readers seem to rely on heuristics a lot, and those may not be intuitive, I think it would be great if there is a JS way to hook up to those cool a11y notifications that are shown in the log, and if we could then hook that up to our mozmill tests, either in the existing tab suite, or some other one. The test could then codify what we believe are the proper a11y events, ideally as told to us by someone who knows what a usable Thunderbird sounds like to a screen reader Which is to say, I have no clue if we need to generate a focus event; I would just write the test then wail on the thunderbird code until the test passes.
(In reply to Andrew Sutherland (:asuth) from comment #20) > Given that screen-readers seem to rely on heuristics a lot, and those may > not be intuitive, No heuristics at all here. If we get a focus event for the document, we'll read the document. If we don't, we don't know the focus has changed, so we don't read the document.
(In reply to David :Bienvenu from comment #15) > we re-use docshells when switching messages, What does resuing docshells means technically? > but if you're coming from a tab > that's not displaying a document, it might be that we just switch decks. do you use toolkit tabbox widget or do you have own one? It seems that tabbox binding should take care of focus event.
(In reply to Andrew Sutherland (:asuth) from comment #20) > (In reply to David :Bienvenu from comment #18) > > Thx, Andrew. So, *should* we be synthesizing a focus event in this case, as > > a notification? If so, which code should Alexander look at? > > So, (regardless of this specific bug) Thunderbird needs to perform some kind > of focusing in tabmail.xml. It is not doing so, but mailTabs.js is doing > its own focus maintenance (and that is likely what would have changed > between Thunderbird 3.1 and later versions). Why can't tabmail.xml XBL widgets rely on focus logic of toolkit's tabbox? Why kind of focus maintaince does mailTabs.js provide? > Given that screen-readers seem to rely on heuristics a lot, and those may > not be intuitive, I think it would be great if there is a JS way to hook up > to those cool a11y notifications that are shown in the log, and if we could > then hook that up to our mozmill tests, either in the existing tab suite, or > some other one. The test could then codify what we believe are the proper > a11y events, ideally as told to us by someone who knows what a usable > Thunderbird sounds like to a screen reader accessible/src tests has some specific Firefox a11y tests like http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/events/test_focus_browserui.xul. That could be done for Thunderbird as well. > Which is to say, I have no clue if we need to generate a focus event; I > would just write the test then wail on the thunderbird code until the test > passes. this is interesting idea but I don't have any implementation ideas, it looks sophisticated. Usually we create a11y tests for particular testcase when we know what exactly is expected from it.
(In reply to alexander surkov from comment #22) > > What does resuing docshells means technically? It means we don't create a new docshell when opening a tab, which makes us different from the browser. > do you use toolkit tabbox widget or do you have own one? It seems that > tabbox binding should take care of focus event. We very much have our own, as I understand it. But this isn't code I deal with much so people cc'ed should jump in if I'm wrong.
(In reply to David :Bienvenu from comment #24) > (In reply to alexander surkov from comment #22) > > > > What does resuing docshells means technically? > It means we don't create a new docshell when opening a tab, which makes us > different from the browser. Can you give more information please? Usually in Firefox you load URL and DOM document, presshell, docshell are created (destroyed) automatically and that's handled well by accessibility. For example, accessibility listens for presshell destroy to destroy the accessible document. If Gecko allows to do something different and that's you use in Thunderbird then accessibility might need to have special processing for this.
I'm not sure what more information I can give. Instead of creating a new docshell every time a message is loaded, we re-use the docshell, and load a new URL into it. We've always done this. If you set a breakpoint here, you'll see that we use the same docshell for every message we load in that 3-pane window. http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessenger.cpp#526
(In reply to alexander surkov from comment #23) > Why can't tabmail.xml XBL widgets rely on focus logic of toolkit's tabbox? Thunderbird's tabmail.xml is derived from an older version of toolkit's. It's a fork because, as bienvenu alludes to, all the mail tabs aren't actually on their own tabs. We use a single panel for all mail tabs, and on tab switch: 1) re-attach the nsMsgDBView to the <tree> widget 2) if the message pane is visible, re-trigger display with whatever message was visible > Why kind of focus maintaince does mailTabs.js provide? see saveFocus, restoreFocus: http://mxr.mozilla.org/comm-central/source/mail/base/content/mailTabs.js#536
(In reply to alexander surkov from comment #23) > Why can't tabmail.xml XBL widgets rely on focus logic of toolkit's tabbox? er, but to clarify, we do use xul:tabbox, but not the browser's tabbrowser. And that because we're not actually switching panels when we switch between mail tabs, the XUL tab stuff is probably not getting a chance to fire, etc.
(In reply to Andrew Sutherland (:asuth) from comment #27) > > Why kind of focus maintaince does mailTabs.js provide? > > see saveFocus, restoreFocus: > http://mxr.mozilla.org/comm-central/source/mail/base/content/mailTabs.js#536 btw, I wasn't able to hit these methods (I put both dump and alert on debug builds). Neither openTab or swithTab in mailTabs.js file. I can see that methods from tabmail.xml binding are triggered. Should tabmail.xml manage the focus actually? That's be similar what toolkit tabbox.xml does.
(In reply to alexander surkov from comment #29) > (In reply to Andrew Sutherland (:asuth) from comment #27) > > > > Why kind of focus maintaince does mailTabs.js provide? > > > > see saveFocus, restoreFocus: > > http://mxr.mozilla.org/comm-central/source/mail/base/content/mailTabs.js#536 > > btw, I wasn't able to hit these methods (I put both dump and alert on debug > builds). Neither openTab or swithTab in mailTabs.js file. due to some reason it doesn't pick up changes from mailTabs.js (objdir/mozilla/dist/bin/chrome/messenger) but chrome://messenger/content/mailTabs.js shows changed version. What is the faster way to change mailTabs.js and see what happens?
(In reply to alexander surkov from comment #30) > due to some reason it doesn't pick up changes from mailTabs.js > (objdir/mozilla/dist/bin/chrome/messenger) but > chrome://messenger/content/mailTabs.js shows changed version. What is the > faster way to change mailTabs.js and see what happens? That's confusing. If you modified that file, it should have worked out. Are you sure your Thunderbird process actually terminated rather than activating an existing process that had the JS file cached in a fastload? The default for Thunderbird builds switched on 12/15/2010 to automatically symlinking on Mac and Linux and copying files on Windows, so there should not be many extra steps involved. (And it sounds like you modified the target file anyways...) You could try setting the following prefs to true if things are still wacky: nglayout.debug.disable_xul_cache nglayout.debug.disable_xul_fastload
Any chance of further investigation on this soon? This bug is a real problem for screen reader users of Thunderbird. It's enough to make me recommend Thunderbird 3.x for most users, which is not a good state of affairs. :)
Trev, maybe you would like to look at it.
bug 799133 might be a cause of this one, somebody needs to try this bug again
Flags: needinfo?
(In reply to alexander :surkov from comment #35) > bug 799133 might be a cause of this one, somebody needs to try this bug again actually bug 612830 might be a cause, bug 799133 is similar to this one
Flags: needinfo?
Flags: needinfo?
This bug still exists (no change) in Thunderbird nightly 20121122030337.
Flags: needinfo?
Jamie, when I do steps from comment #4 I can see focus event fired on document accessible of opened message. Can you confirm that?
(In reply to alexander :surkov from comment #38) > Jamie, when I do steps from comment #4 I can see focus event fired on > document accessible of opened message. Can you confirm that? No. I tested with AccProbe and I definitely don't see a focus event on the document. Note that you should try opening a message you've already opened recently, as caching probably makes this bug more likely.
I can see focus events either by DOMi or accProbe. Actually I see two focus events for the same document. Very rare I get one focus event and no events (that's what you observe). The issue is intermittent for me.
(In reply to alexander :surkov from comment #40) > I can see focus events either by DOMi or accProbe. Actually I see two focus > events for the same document. Very rare I get one focus event and no events > (that's what you observe). Really strange. This is not unique to me; every NVDA+Tb user I know has confirmed this. Are you opening in a new tab (as opposed to a new window)? How big is the message you're opening? Smaller messages will be cached entirely, so they load immediately and are thus more likely to reproduce the problem.
(In reply to James Teh [:Jamie] from comment #41) > (In reply to alexander :surkov from comment #40) > > I can see focus events either by DOMi or accProbe. Actually I see two focus > > events for the same document. Very rare I get one focus event and no events > > (that's what you observe). > Really strange. This is not unique to me; every NVDA+Tb user I know has > confirmed this. Are you opening in a new tab (as opposed to a new window)? > How big is the message you're opening? Smaller messages will be cached > entirely, so they load immediately and are thus more likely to reproduce the > problem. It's open in new tab, it was small messages (like bugzilla emails). I'm not sure whether Thunderbird nightly uses Mozilla nightly sources of the same date (i.e. in case if bug 612830 was a cause of this and it's not a part of the build you tried). I remember that it was permanent issue when I tried it before.
Problem persists for me in nightly 20121125030320.
I've built debug build and I see the issue reliably. We receive DOM focus event for not accessible document, that's timing issue I think.
Log: A11Y FOCUS: DOM focus; 33:55.573 { [not accessible] Target: 0975A148, document [contained by not accessible document]: DOM document: 0975A148, acc document: 00000000 uri: imap://surkov%2Ealexander%40gmail%2Ecom@imap.googlemail.com:993/fetch%3EUID%3E/INBOX%3E183862 docshell busy: 'busy', 'page loading'; content document docshell hierarchy, parent: 0559C584, root: 0559C584, is tab document: yes; doc state: loading, not initial, not showing, visible, active, not resource, has no role content presshell: 07C3CA38, is not destroying, root scroll frame: 00000000 load group: 07874540, parent id: 05611E90 parent uri: chrome://messenger/content/messenger.xul } there's no root scroll frame, we reject to create an accessible when there's no root frame. I think that's a cause since I don't see anything else that may fail. I think we require root frame because we add scroll event listeners on it. So we have two options here: 1) create document earlier and add scroll listeners when we get some content (create a11y tree) 2) collect important notifications like DOM focus somewhere and process them later when document is created Trev, opinion?
I think creating documents earlier is probably nicer.
Attached patch patch (obsolete) — — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #688662 - Flags: review?(trev.saunders)
Attached patch patch2 (obsolete) — — Splinter Review
previous patch caused a earlier document creation which caused an intermittent failure: 6654 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/pivot/test_virtualcursor.html | no nsIAccessibleCursorable support in child document this fails when document doesn't have a parent document yet. So I moved initialization into DoInitialUpdate() where documents are connected already.
Attachment #688662 - Attachment is obsolete: true
Attachment #688662 - Flags: review?(trev.saunders)
Attachment #689592 - Flags: review?(trev.saunders)
Attached patch patch3 (obsolete) — — Splinter Review
patch2 was from different bug, filing a correct one (sorry for the spam)
Attachment #689592 - Attachment is obsolete: true
Attachment #689592 - Flags: review?(trev.saunders)
Attachment #689594 - Flags: review?(trev.saunders)
Comment on attachment 689594 [details] [diff] [review] patch3 this one is not correct one too (sorry)
Attachment #689594 - Attachment is obsolete: true
Attachment #689594 - Flags: review?(trev.saunders)
Attached patch patch4 — — Splinter Review
Attachment #689597 - Flags: review?(trev.saunders)
(In reply to alexander :surkov from comment #48) > Created attachment 689592 [details] [diff] [review] > patch2 > > previous patch caused a earlier document creation which caused an > intermittent failure: > > 6654 ERROR TEST-UNEXPECTED-FAIL | > chrome://mochitests/content/a11y/accessible/pivot/test_virtualcursor.html | > no nsIAccessibleCursorable support in child document > > this fails when document doesn't have a parent document yet. So I moved > initialization into DoInitialUpdate() where documents are connected already. actually it was caused by uninitialized mDocFlags member. But in either case I think it makes sense to wait until DoInitialUpdate(), pivot traversal doesn't make a lot of sense until document is connected.
Comment on attachment 689597 [details] [diff] [review] patch4 > mDocument->DoInitialUpdate(); > > NS_ASSERTION(mContentInsertions.Length() == 0, > "Pending content insertions while initial accessible tree isn't created!"); > } > >+ // Initialize scroll support if needed. >+ if (!(mDocument->mDocFlags & DocAccessible::eScrollInitialized)) >+ mDocument->AddScrollListener(); could we do this during DoInitialUpdate() ? is there a reason you don't add a test?
Attachment #689597 - Flags: review?(trev.saunders) → review+
(In reply to Trevor Saunders (:tbsaunde) from comment #53) > >+ // Initialize scroll support if needed. > >+ if (!(mDocument->mDocFlags & DocAccessible::eScrollInitialized)) > >+ mDocument->AddScrollListener(); > > could we do this during DoInitialUpdate() ? DoInitialUpdate doesn't seem guarantee a ready root frame > is there a reason you don't add a test? no ideas how to make it. do you?
(In reply to alexander :surkov from comment #54) > (In reply to Trevor Saunders (:tbsaunde) from comment #53) > > > >+ // Initialize scroll support if needed. > > >+ if (!(mDocument->mDocFlags & DocAccessible::eScrollInitialized)) > > >+ mDocument->AddScrollListener(); > > > > could we do this during DoInitialUpdate() ? > > DoInitialUpdate doesn't seem guarantee a ready root frame > > > is there a reason you don't add a test? > > no ideas how to make it. do you? not off hand, maybe look at thunderbird stuff for idea how it happens for them?
I'd better save time for something else. The test of this kind can eat a significant amount of time with no luck.
https://hg.mozilla.org/mozilla-central/rev/af2c780aa931
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
I'd be great if somebody can verify the fix.
Verified fixed in Thunderbird 20.0a1 20130102030909. Thanks heaps!
Status: RESOLVED → VERIFIED
thank you, Jamie.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: