Closed
Bug 468497
Opened 16 years ago
Closed 6 years ago
Switching tabs using Ctrl+Tab does not generate an accessible focus event on the newly focused control.
Categories
(Thunderbird :: Toolbars and Tabs, defect)
Thunderbird
Toolbars and Tabs
Tracking
(thunderbird_esr60 fixed, thunderbird62 wontfix, thunderbird63 fixed)
RESOLVED
FIXED
Thunderbird 63.0
People
(Reporter: MarcoZ, Assigned: Jamie)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file, 3 obsolete files)
59 bytes,
text/x-review-board-request
|
MarcoZ
:
review+
jorgk-bmo
:
approval-comm-esr60+
|
Details |
STR: 1. Open Shredder. 2. With your default Inbox folder open, right-click a different folder and choose "Open in new tab". 3. Focus the message list. 4. Press Ctrl+Tab. Expected: Contents of message list changes to your Inbox. Screen reader should notify the user. Actual: Contents changes, but screen reader does not receive notification. Only after hitting up and down arrow once is there a refresh in content.
Reporter | ||
Updated•15 years ago
|
Flags: blocking-thunderbird3?
Comment 1•15 years ago
|
||
Marco, what do we need to do to fix this bug? Is it just a matter of calling .focus() on the DOM element corresponding to the tab panel?
Reporter | ||
Comment 2•15 years ago
|
||
(In reply to comment #1) > Marco, what do we need to do to fix this bug? Is it just a matter of calling > .focus() on the DOM element corresponding to the tab panel? No, because the focus never leaves the tree table. The question is: Do you reuse the same tree table and simply repopulate it with new content, which is what seems to be happening, or do you create new tree tables each time? A trick Aaron Leventhal once told me is this: Fire a DOMMenuItemActive event on the selected tree child once the tabs are switched. A11y code translates this into a regular focus event if fired on anything other than a real menu item.
Comment 3•15 years ago
|
||
Sounds pretty painful from accessibility pov.
Component: Mail Window Front End → Toolbars and Tabs
Flags: blocking-thunderbird3? → blocking-thunderbird3+
QA Contact: front-end → toolbars-tabs
Updated•15 years ago
|
Target Milestone: --- → Thunderbird 3.0rc1
Comment 4•15 years ago
|
||
Pulling this in to beta 4. The fix sounds easy but the mozmill test implications will require investigation. Ideally I would like some form of accessibility 'listener' that we could use to support assertions about events having been generated that are appropriate for screen readers and the like. We can take a fix for this without a unit test if someone wants to provide one. Tsk/Standard8, have you perchance seen any signs of this already existing for mozmill?
OS: Windows Vista → All
Hardware: x86 → All
Target Milestone: Thunderbird 3.0rc1 → Thunderbird 3.0b4
Assignee | ||
Comment 5•15 years ago
|
||
This bug also applies (i.e. a focus event is not fired) when pressing enter to open a message with "Open messages in a new tab" configured. Closing a message tab also fails to fire an appropriate focus event. In these cases, pressing up/down arrows when returning to a message list tab often isn't enough to restore focus; one has to press tab once or twice.
Assignee | ||
Comment 6•15 years ago
|
||
(In reply to comment #5) > This bug also applies (i.e. a focus event is not fired) when pressing enter to > open a message with "Open messages in a new tab" configured. Just saw that this is covered by bug 501308.
Comment 7•15 years ago
|
||
We're getting to the point in the release cycle where we have to start letting go of bugs which don't effect a sufficiently large percentage of users. As much as we want this for Thunderbird 3, I don't think we'd let it hold back all the other goodness that's landed since Tb2 if it were the last bug standing. Marking as blocking-, wanted+, in the hopes that someone does find the bandwidth to come up with a patch.
Flags: wanted-thunderbird3+
Flags: blocking-thunderbird3-
Flags: blocking-thunderbird3+
Assignee | ||
Comment 8•14 years ago
|
||
Any chance of a fix for this in 3.1? Is Marco's solution (though arguably a bit of a hack) as described in comment 2 (using DOMMenuItemActive) acceptable? Interestingly, the focused state is set on the new active item as it should be, but no focus event is fired.
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.4) Gecko/20100608 Thunderbird/3.1 this behaviour persists. Furthermore if browsing through messages by <ctrl><tab> or <ctrl><shift><tab> you see the proper messages but the "tab focus" is never displayed or moved unless the number of messages is less or equal to the number that fit in the current windows width. Advise please.
Comment 10•12 years ago
|
||
(In reply to Michał from comment #9) > Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.4) Gecko/20100608 > Thunderbird/3.1 > > this behaviour persists. Furthermore if browsing through messages by > <ctrl><tab> or <ctrl><shift><tab> you see the proper messages but the "tab > focus" is never displayed or moved unless the number of messages is less or > equal to the number that fit in the current windows width. > Advise please. I filed that as bug 725469, maybe two sides of the same coin. It's unbelievable. I still see this behaviour in TB10.
Comment 11•12 years ago
|
||
See also: 695285, 577655.
Comment 12•12 years ago
|
||
(In reply to Michał from comment #11) > See also: 695285, 577655. Thanks, Michał, for pointing out some connections and describing the problem of comment 9, which is now bug 725469. Yes, these bugs/rfes also refer to the tab bar, and insofar are related. Otherwise, bug 695285 is unrelated to this bug, and I forward-duplicated bug 577655 against more concise bug 725469, which might well be related or even a duplicate of this one.
Updated•12 years ago
|
No longer blocks: tbird3access
Assignee | ||
Comment 13•10 years ago
|
||
Inform FocusManager when the tree's view changes. This seems to work... sometimes. I've no idea why it doesn't work all the time. I've confirmed that the item I'm passing definitely isn't the same as the previous item FocusManager knew about, but it still doesn't fire an event in some cases.
Assignee | ||
Comment 14•10 years ago
|
||
I've also confirmed that in all cases, the right item is definitely getting passed to FocusManager::ActiveItemChanged. It always has a state of invisible at this point, even in the cases where it works. I'm starting to think the event somehow gets blocked because the tree hasn't been updated visually yet. I tried moving the TreeViewChanged a11y call in nsTreeBodyFrame to ReflowFinished (with a flag of course), but it doesn't seem to get called. This is probably a bad idea anyway. :) Frustratingly, I can't use a11y logging to debug this because when I do, the bug goes away, which suggests it's definitely timing related.
Assignee | ||
Comment 15•10 years ago
|
||
Bah. Patch accidentally included unrelated stuff. Sorry.
Assignee | ||
Updated•10 years ago
|
Attachment #8474043 -
Attachment is obsolete: true
Reporter | ||
Comment 16•10 years ago
|
||
Neil, any thoughts about the problems/questions raised by Jamie in comment #14?
Flags: needinfo?(enndeakin)
Comment 17•10 years ago
|
||
(In reply to James Teh [:Jamie] from comment #14) > I'm starting to think the event somehow gets blocked because the tree hasn't > been updated visually yet. I tried moving the TreeViewChanged a11y call in > nsTreeBodyFrame to ReflowFinished (with a flag of course), but it doesn't > seem to get called. This is probably a bad idea anyway. :) > A reflow will only occur if the tree needs to change its size or layout. TreeViewChanged is only called when the view is changed. Is the view being changed here, or just the data returned by the view?
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 18•10 years ago
|
||
(In reply to Neil Deakin from comment #17) > TreeViewChanged is only called when the view is changed. Is the view being > changed here, or just the data returned by the view? I'm pretty sure the view is being changed. TreeViewChanged does seem to get called. As noted in comment 14, I even know that I'm retrieving the item that should get focus, but it's always invisible at the time TreeViewChanged is called. I haven't worked out yet why accessibility sometimes fires the focus event and other times refuses, but the fact that it's invisible at that point means something is clearly not ready at that point.
Comment 19•9 years ago
|
||
Neil, any further thoughts relative to comment 18?
Severity: normal → major
Flags: needinfo?(enndeakin)
Target Milestone: Thunderbird 3.0b4 → ---
Reporter | ||
Comment 21•6 years ago
|
||
Jamie's patch, rebased to current code base. I can confirm that this fixes the bug. If I have two folders open and focus is in both messages lists, ctrl+tab will reliably speak the newly focused item in the new tab. Note that the tab itself is not spoken, but I think that's more a screen reader side of thing than ours. Also if I open a message from either, so it is wedged between the two folder trees, ctrl+tabbing between all three tabs will always speak the focused item. Jamie, what do you think, should we take this? Asking Surkov for review since we kinda both worked on this. ;)
Attachment #8474093 -
Attachment is obsolete: true
Flags: needinfo?(jteh)
Attachment #8989726 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 22•6 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #21) > Note that the tab itself is not spoken, but I think > that's more a screen reader side of thing than ours. From a screen reader perspective, there's nothing that would cause the tab to be reported. IIRC, message list tabs don't have their own tab panel/property page accessible in the hierarchy, so the focus ancestry doesn't change and thus there's no new "context" to report aside from the newly focused item. You could perhaps argue a screen reader should watch for selection events on tabs, but that would probably need to be app specific. In a super ideal world, each tab would have its own container and the ancestry would thus change, but that's a massive change and would probably prevent reuse of the tree. > Jamie, what do you think, should we take this? If it fixes the bug, I don't see why not. Ideally, we'd have some kind of test for it, but I think writing a test which covers this is going to be pretty complicated and I'm not sure it's worth the effort given that this has only ever been seen in Thunderbird. We're using XUL less and less these days.
Flags: needinfo?(jteh)
Reporter | ||
Comment 23•6 years ago
|
||
Comment on attachment 8989726 [details] [diff] [review] Rebased patch As this is Jamie's patch, which I just rebased, I am giving this an r+ as a module peer. Jamie, feel free to land at your discretion. Since this is Gecko, and only concerning Thunderbird, it'll probably be hard to get it backported to 60ESR, which the next TB release will use, but going forward, this should then be fixed for TB users.
Attachment #8989726 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 24•6 years ago
|
||
Happy to get this landed. I agree uplift to 60 ESR is probably not something we can justify, though.
Keywords: checkin-needed
Reporter | ||
Comment 25•6 years ago
|
||
The patch currently has no author and commit message, so you may want to update it with that.
Assignee | ||
Comment 26•6 years ago
|
||
Ug, forgot about that. Thanks for the heads up. I might re-submit this as a mozreview request to make my life easier if you don't mind r+ing it again.
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8989726 -
Attachment is obsolete: true
Reporter | ||
Comment 28•6 years ago
|
||
mozreview-review |
Comment on attachment 8989909 [details] Bug 468497: Inform the accessibility FocusManager when a XUL tree's view changes. https://reviewboard.mozilla.org/r/254916/#review261796
Attachment #8989909 -
Flags: review?(mzehe) → review+
Comment 29•6 years ago
|
||
(In reply to James Teh [:Jamie] from comment #24) > Happy to get this landed. I agree uplift to 60 ESR is probably not something > we can justify, though. But if it only affects Thunderbird, then what's the harm in uplift?
Comment 30•6 years ago
|
||
Thunderbird uses a branch off mozilla-central 60, with customizations, too so you can ask to uplift it to that branch too instead if not accepted to mozilla-central directly.
Assignee | ||
Comment 31•6 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #29) > But if it only affects Thunderbird, then what's the harm in uplift? The code relates to XUL trees, so it could theoretically affect Firefox. I've just never seen this bug in Firefox, but that doesn't mean there isn't a potential for unforeseen regressions. The risk is low, but it's still there, which makes it hard to justify uplift to mozilla-central. (In reply to Magnus Melin from comment #30) > Thunderbird uses a branch off mozilla-central 60, with customizations, too > so you can ask to uplift it to that branch too instead if not accepted to > mozilla-central directly. Ah. Is there a flag to request this directly? I'm very reluctant to ask for uplift to m-c.
Comment 32•6 years ago
|
||
Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4fd60abd9726 Inform the accessibility FocusManager when a XUL tree's view changes. r=MarcoZ
Comment 33•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4fd60abd9726
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
Updated•6 years ago
|
Assignee: nobody → jteh
Comment 34•6 years ago
|
||
(In reply to James Teh [:Jamie] from comment #31) > Ah. Is there a flag to request this directly? I'm very reluctant to ask for > uplift to m-c. Not directly, but you can ask for approval‑comm‑esr60 uplift and just add info to the comment.
Assignee | ||
Comment 35•6 years ago
|
||
Comment on attachment 8989909 [details] Bug 468497: Inform the accessibility FocusManager when a XUL tree's view changes. [Approval Request Comment] Regression caused by (bug #): None. User impact if declined: Screen reader users will not receive any notification when they switch between folder tabs with the keyboard, so they won't know what message is currently focused. Testing completed (on c-c, etc.): Another user/developer verified that this patch fixes the bug in comment 21. Risk to taking this patch (and alternatives if risky): Low risk. Only affects accessibility, and only when a tree is completely changed (its view changes). Note that this patch is a mozilla-* patch, not a comm-* patch. I haven't requested uplift to Mozilla ESR because while it's low risk, I don't know of any manifestation of this bug in Firefox, so I can't justify the uplift for mozilla ESR. See comment 30 and comment 34.
Attachment #8989909 -
Flags: approval-comm-esr60?
Comment 36•6 years ago
|
||
Comment on attachment 8989909 [details] Bug 468497: Inform the accessibility FocusManager when a XUL tree's view changes. Something is wrong here. This as an M-C bug and you're asking for inclusion in TB 60 (and consequently also 62) via a release branch using comm approval flags. Hmm. OK, I'll do it.
Attachment #8989909 -
Flags: approval-comm-esr60?
Attachment #8989909 -
Flags: approval-comm-esr60+
Attachment #8989909 -
Flags: approval-comm-beta+
Assignee | ||
Comment 37•6 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #36) > Something is wrong here. This as an M-C bug and you're asking for inclusion > in TB 60 (and consequently also 62) via a release branch using comm approval > flags. Hmm. I tried to explain the reasons for this in the uplift request, and I requested comm approval based on the advice in comment 34. I don't know of another way to request uplift to the comm specific fork of mozilla 60 ESR. Apologies if I did this the wrong way; I'm not familiar with the Thunderbird release process for m-c changes given that it's quite separate from Firefox. In terms of Firefox, this probably wouldn't be something I'd uplift to 60. However, given that Thunderbird releases are less regular, I didn't want users to have to wait so long for the fix. If this is all just too messy, that's fine and reasonable; we can just wait until the next Thunderbird after 60.
Comment 38•6 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr60/rev/8c585f7d67f5636dc3eef2cbd5b71052b570fdd1 (THUNDERBIRD_60_VERBRANCH)
Updated•6 years ago
|
status-thunderbird62:
--- → affected
status-thunderbird63:
--- → fixed
status-thunderbird_esr60:
--- → fixed
Comment 39•6 years ago
|
||
Trick to get this off the books for uplift since my query looks for releases/comm-esr60/rev ;-)
Updated•6 years ago
|
Updated•6 years ago
|
Attachment #8989909 -
Flags: approval-comm-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•