missing focus event when opening a message in Thunderbird

VERIFIED FIXED in mozilla20

Status

()

Core
Disability Access APIs
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Mike Gorse, Assigned: surkov)

Tracking

(Blocks: 1 bug, {regression})

10 Branch
mozilla20
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
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: 472809
(Assignee)

Updated

7 years ago
Blocks: 375743
No longer blocks: 472809
(Assignee)

Comment 1

7 years ago
Fernando, yours one?

Comment 2

7 years ago
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
(Reporter)

Comment 3

7 years ago
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.
OS: Linux → All

Comment 4

7 years ago
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

Updated

7 years ago
Keywords: regression

Comment 5

7 years ago
Related NVDA issue ticket: http://www.nvda-project.org/ticket/1622
(Assignee)

Comment 6

7 years ago
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
(Assignee)

Updated

7 years ago
Target Milestone: mozilla7 → ---

Comment 7

7 years ago
(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.

Updated

7 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 8

7 years ago
Created attachment 567024 [details]
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.

Comment 9

7 years ago
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.

Comment 10

7 years ago
(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.
(Assignee)

Comment 11

7 years ago
(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.

Comment 12

7 years ago
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.
(Assignee)

Comment 13

7 years ago
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?
(Assignee)

Comment 14

7 years ago
(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.

Comment 15

7 years ago
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.

Comment 18

7 years ago
Thx, Andrew. So, *should* we be synthesizing a focus event in this case, as a notification? If so, which code should Alexander look at?

Comment 19

7 years ago
(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.

Comment 21

7 years ago
(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.
(Assignee)

Comment 22

7 years ago
(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.
(Assignee)

Comment 23

7 years ago
(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.

Comment 24

7 years ago
(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.
(Assignee)

Comment 25

7 years ago
(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.

Comment 26

7 years ago
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.
(Assignee)

Comment 29

7 years ago
(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.
(Assignee)

Comment 30

7 years ago
(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
Duplicate of this bug: 674180

Comment 33

6 years ago
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. :)
(Assignee)

Comment 34

6 years ago
Trev, maybe you would like to look at it.
(Assignee)

Comment 35

6 years ago
bug 799133 might be a cause of this one, somebody needs to try this bug again
Flags: needinfo?
(Assignee)

Comment 36

6 years ago
(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?
(Assignee)

Updated

6 years ago
Flags: needinfo?

Comment 37

6 years ago
This bug still exists (no change) in Thunderbird nightly 20121122030337.
Flags: needinfo?
(Assignee)

Comment 38

6 years ago
Jamie, when I do steps from comment #4 I can see focus event fired on document accessible of opened message. Can you confirm that?

Comment 39

6 years ago
(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.
(Assignee)

Comment 40

6 years ago
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.

Comment 41

6 years ago
(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.
(Assignee)

Comment 42

6 years ago
(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.

Comment 43

6 years ago
Problem persists for me in nightly 20121125030320.
(Assignee)

Comment 44

6 years ago
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.
(Assignee)

Comment 45

6 years ago
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.
(Assignee)

Comment 47

6 years ago
Created attachment 688662 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #688662 - Flags: review?(trev.saunders)
(Assignee)

Comment 48

6 years ago
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.
Attachment #688662 - Attachment is obsolete: true
Attachment #688662 - Flags: review?(trev.saunders)
Attachment #689592 - Flags: review?(trev.saunders)
(Assignee)

Comment 49

6 years ago
Created attachment 689594 [details] [diff] [review]
patch3

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)
(Assignee)

Comment 50

6 years ago
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)
(Assignee)

Comment 51

6 years ago
Created attachment 689597 [details] [diff] [review]
patch4
Attachment #689597 - Flags: review?(trev.saunders)
(Assignee)

Comment 52

6 years ago
(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+
(Assignee)

Comment 54

6 years ago
(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?
(Assignee)

Comment 56

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(Assignee)

Comment 59

6 years ago
I'd be great if somebody can verify the fix.

Comment 60

6 years ago
Verified fixed in Thunderbird 20.0a1 20130102030909. Thanks heaps!
Status: RESOLVED → VERIFIED
(Assignee)

Comment 61

6 years ago
thank you, Jamie.
You need to log in before you can comment on or make changes to this bug.