Closed Bug 568916 Opened 14 years ago Closed 8 years ago

Add pref to enable full accessibility

Categories

(Core :: Disability Access APIs, defect)

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

Tracking

()

Status:
RESOLVED INCOMPLETE

People

(Reporter: davidb, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [auto-closed:inactivity])

Our accessibility support in Firefox is lazily instantiated (when someone requests an accessible object). There are two main triggers for this: 1. Accessibility/Assistive Technology (AT) 2. Others (Anti-spyware, Tablet PC) Our accessibility code is targeted towards 1, and involves some serious computation at times. The balance shifts to accessibility and correctness over performance. Users with the second class of trigger and even some users of the first, probably do not want to suffer the performance hit. Bug 538530 (regarding Tablet PCs) introduced a preference to force disable accessibility on Windows. We should probably consider adding a new middle ground default setting. So something like: 1. Regular Accessibility 2. Full Accessibility 3. No Accessibility For most users 1 would be the default, unless there is indication 2 should be the default (perhaps a registry key, or some other indicator). Users would be able to set 1, 2 or 3 manually. Yes, there is a cost to adding complexity here, which we should weight against the cost of hurting performance unnecessarily. Note this bug does not imply we would discontinue work on our a11y perf bugs.
Regular versus full accessibility really needs to be defined. As a screen reader user, I am probably going to need full accessibility. A registry key is not sufficient, as this breaks use by portable screen readers. If you make this a preference screen reader users must set, you will lose a lot of new screen reader users very quickly and possibly some existing ones as well. I can't speak for users of other types of AT, but I'd wager the same is true. Remember, we don't have to do any of this for IE. (Granted, supporting IE is harder work on the AT side, but the statement still stands.) Note that if something asks for an accessible object, you cannot really make any assumptions about what it may or may not want. The whole point of asking for an accessible object is that the caller wants to use it in some way. Otherwise, it wouldn't have asked. What it actually wants is undefined. Is this something you envisage as a permanent solution or just a temporary one until these perf issues are sorted?
(In reply to comment #1) > Is this something you envisage as a permanent solution or just a temporary one > until these perf issues are sorted? I believe this must be temporary solution. Personally I wouldn't like to force screen readers users to make anything to get full a11y support. I hope that's not going to happen ever. Btw, accessibility cutting might require significant time especially if default setting would be a restricted a11y version because it must be tested and made sure it doesn't regress most of web pages rendering by AT.
(In reply to comment #2) > (In reply to comment #1) > > > Is this something you envisage as a permanent solution or just a temporary one > > until these perf issues are sorted? > > I believe this must be temporary solution. > > Personally I wouldn't like to force screen readers users to make anything to > get full a11y support. I hope that's not going to happen ever. I agree but I think we're willing to take temporary solutions if they can protect screen reader users from having an unresponsive browser. > Btw, accessibility cutting might require significant time especially if default > setting would be a restricted a11y version because it must be tested and made > sure it doesn't regress most of web pages rendering by AT. Since the bogus invocation of a11y is happening on Windows I think we would make non-coalescence-a11y default there except we should be able to detect cases where someone is using Jaws or NVDA, and for those users, make full a11y the default. But then, they are still faced with unresponsiveness... which they can disable if they choose. It does add triage complexity -- I know.
(In reply to comment #3) > I agree but I think we're willing to take temporary solutions if they can > protect screen reader users from having an unresponsive browser. Do we have any complaints about unresponsive browser from screen reader users? Isn't unresponsive browser all about tablet pc? > Since the bogus invocation of a11y is happening on Windows I think we would > make non-coalescence-a11y default there except we should be able to detect > cases where someone is using Jaws or NVDA, and for those users, make full a11y > the default. But then, they are still faced with unresponsiveness... which they > can disable if they choose. Had you have confirmed the root problem in events coalescence? Sometimes I have a feeling you're about to take important decisions just to try.
(In reply to comment #4) > (In reply to comment #3) > > > I agree but I think we're willing to take temporary solutions if they can > > protect screen reader users from having an unresponsive browser. > > Do we have any complaints about unresponsive browser from screen reader users? > Isn't unresponsive browser all about tablet pc? We have some from Orca at least. > > Since the bogus invocation of a11y is happening on Windows I think we would > > make non-coalescence-a11y default there except we should be able to detect > > cases where someone is using Jaws or NVDA, and for those users, make full a11y > > the default. But then, they are still faced with unresponsiveness... which they > > can disable if they choose. > > Had you have confirmed the root problem in events coalescence? Yes. I profiled and for the mozilla hacks examples the top two heavy hitters where CoalesceEvents and ApplyToSiblings. > Sometimes I have a feeling you're about to take important decisions just to > try. Sorry it looks that way. I really do consider this quite serious which is why I haven't started a patch on this bug yet.
Marco, as per IRC discussion with Alexander, we are looking into the impact of turning coalescence off. Can you comment out the CoalesceEvents call in nsAccEventQueue::Push and test performance and any other side effects? I'll post a link to a try build when I have one. (In reply to comment #1) > Regular versus full accessibility really needs to be defined. As a screen > reader user, I am probably going to need full accessibility. A registry key is > not sufficient, as this breaks use by portable screen readers. If you make this > a preference screen reader users must set, you will lose a lot of new screen > reader users very quickly and possibly some existing ones as well. I can't > speak for users of other types of AT, but I'd wager the same is true. Remember, > we don't have to do any of this for IE. (Granted, supporting IE is harder work > on the AT side, but the statement still stands.) Thanks Jamie, do you have to do any event throttling for IE?
(In reply to comment #6) > Marco, as per IRC discussion with Alexander, we are looking into the impact of > turning coalescence off. Can you comment out the CoalesceEvents call in > nsAccEventQueue::Push and test performance and any other side effects? Turning off Coalescense for screen readers is definitely not an option. I tried the MozHacks testcase and the one from bug 522847 with a local build with that change. In both cases, the number of refreshes causes NVDA to slow to a halt, and the only way to get it back is to kill the Firefox process. With coalescing, I at least get response back after 1 to 2 minutes or so. Also, over-all performance feels more sluggish when loading other web sites. For example, loading blindcooltech.com takes about 2 seconds longer than with coalescing.
(In reply to comment #7) > (In reply to comment #6) > > Marco, as per IRC discussion with Alexander, we are looking into the impact of > > turning coalescence off. Can you comment out the CoalesceEvents call in > > nsAccEventQueue::Push and test performance and any other side effects? > > Turning off Coalescense for screen readers is definitely not an option. I tried > the MozHacks testcase and the one from bug 522847 with a local build with that > change. In both cases, the number of refreshes causes NVDA to slow to a halt, > and the only way to get it back is to kill the Firefox process. With > coalescing, I at least get response back after 1 to 2 minutes or so. That makes turning it off by default for everyone difficult. I guess we could try the screen reader detection approach (or something else). > Also, over-all performance feels more sluggish when loading other web sites. > For example, loading blindcooltech.com takes about 2 seconds longer than with > coalescing. That's expected, but sad to hear. Jamie do you think any of this is would be fixable on the client side?
(In reply to comment #7) > Also, over-all performance feels more sluggish when loading other web sites. > For example, loading blindcooltech.com takes about 2 seconds longer than with > coalescing. This is because we mostly do not fire events while page is loading. It makes sense to test dynamic web pages.
Aaron had the good idea of adding the tradeoff to the names of the options. So like regular-fast, full-slow, none-fastest.
(In reply to comment #3) > > Personally I wouldn't like to force screen readers users to make anything to > > get full a11y support. I hope that's not going to happen ever. > I agree but I think we're willing to take temporary solutions if they can > protect screen reader users from having an unresponsive browser. That's great, but I'd rather an unresponsive browser in rare cases than broken accessibility on the majority of web sites that currently work. :) (In reply to comment #6) > Marco, as per IRC discussion with Alexander, we are looking into the impact of > turning coalescence off. Okay. I don't mean to step on anyone's toes here, but I'd like to get a better idea of the history on coalescence. When was it introduced (what Firefox version)? Why was it introduced (i.e. what test cases, bug reports, etc.)? Anecdotally, it just seems like it may be causing more problems than it fixes, so I'd like to have a better understanding. > Thanks Jamie, do you have to do any event throttling for IE? On the virtual buffer side, IE is very different. We have to use DOM events (via COM) to listen for updates. Win events are still used by NVDA itself ot listen for focus changes, state changes, etc., but only really for the focused object. Note that in all virtual buffers (including Gecko), we don't render buffer changes immediately. Rather, we find a common ancestor for all events within a certain time period (I think it's 300 ms) and render that when the timeout expires. (In reply to comment #8) > > Turning off Coalescense for screen readers is definitely not an option. > That makes turning it off by default for everyone difficult. I guess we could > try the screen reader detection approach (or something else). I don't see what the detection approach would solve. That means that some clients are still going to have no Coalescense, so someone will still get a major performance hit. Or are you assuming this issue is specific to NVDA? (That's fine; I just need to understand your perspective.) > Jamie do you think any of this is would be fixable on the client side? That depends where we think the problem is on the client side. :) Marco, did you test this on Windows 64 bit? If so, you may want to re-test with a recent snapshot, as we made some changes which may improve event performance on 64 bit systems.
Yesterday I've landed a patch (bug 570532) that fixes coalescence perf on known cases when a11y is enabled but AT is not running. As well Marco said NVDA feels better with today build. These results make me think no coalescence is not an option today. Next step is to make text change events performant.
AUTO-CLOSED. This bug untouched for over 2000 days. Please reopen if you can confirm the bug and help it progress.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INCOMPLETE
Whiteboard: [auto-closed:inactivity]
You need to log in before you can comment on or make changes to this bug.