394253 - Opening DOM inspector enables accessibility globally

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 337674.  Opening DOM inspector always enables accessibility globally, even if I never inspect the accessibles for anything.  Once that's happened, the app slows down drastically, and I get hangs (bug 394115).

In other words, once I open DOM Inspector the app becomes effectively unusable.

This really needs to be fixed, imo.  I'm downgrading to a build that predates the checkin for bug 337674 for the time being.

Comment 1

[Aaron Leventhal]

The app slows down drastically? That doesn't sound good either.

Comment 2

[Boris Zbarsky [:bzbarsky]]

At least as far as I can tell, yes.  Not for all operations, but some things become very slow.

Comment 3

[Aaron Leventhal]

I wish we had the resources to investigate that right now, but we need to focus on correctness and removing any hangs/crashes.

Comment 4

[Aaron Leventhal]

Anyway, about this bug --

If we fix the hang/lockups, is the app truly unusable? We don't currently have a way to turn accessibility on on a per-Window basis.

This could be a lot of work to fix for 1.9. We still have plenty to do in terms of correcting bugs in the accessibility API support.

Comment 5

[alexander :surkov (:asurkov)]

Looks like a dupe of bug 391798.

Comment 6

[Boris Zbarsky [:bzbarsky]]

No.  This is NOT the same as bug 391798.  I am NOT asking to be able to turn _off_ accessibility.  I'm just asking to not turn it on unless I actually switch to that panel in Inspector.  Right now the mere act of opening an Inspector window turns it on.

> If we fix the hang/lockups, is the app truly unusable?

It's a huge pain to use, yes.  Little things like typeahead find on short pages taking over a second per letter.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> No.  This is NOT the same as bug 391798.  I am NOT asking to be able to turn
> _off_ accessibility.  I'm just asking to not turn it on unless I actually
> switch to that panel in Inspector.  Right now the mere act of opening an
> Inspector window turns it on.

There are views for left panel. When you start to inspect document then DOMi check whether these views are suitable and it starts ally for this. Therefore we should disable ally by default to prevent these checks in DOMi and therefore I said it's the same.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> and it starts ally for this.

Why?  That is, why does deciding whether the accessible tree view is suitable require starting a11y?

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #8)
> > and it starts ally for this.
> 
> Why?  That is, why does deciding whether the accessible tree view is suitable
> require starting a11y?
> 

Those views checks whether accessible is available for the DOM document. Though probably you're right because every DOM document is able to have accessible object and DOMi shouldn't check this.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yeah, that's sort of what I was thinking.

Comment 11

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 12

[Aaron Leventhal]

Comment on attachment 278909 [details] [diff] [review]
patch

I'm the wrong person to review the JS-foo, but yes any nsIDOMDocument can be made accessible.

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #12)
> (From update of attachment 278909 [details] [diff] [review])
> I'm the wrong person to review the JS-foo, but yes any nsIDOMDocument can be
> made accessible.
> 

that's why I asked you to make sure for accessible stuff.

Comment 14

[alexander :surkov (:asurkov)]

Do I need an approval for UI changes in DOMi because Shawn said previously I don't need it still?

Comment 15

[timeless]

oh right, no. but the rules are so confusing that i can never keep them straight.

Comment 16

[alexander :surkov (:asurkov)]

checked in

Comment 17

[Boris Zbarsky [:bzbarsky]]

Alexander, thanks for fixing!

Comment 18

[Martijn Wargers (dead)]

As a sidenote, this seems really handy for stability testing of accessibility code.
Just open the dom inspector one, do some testing and a crash is waiting to happen.
Is is useful to file this type of bugs?

Comment 19

[Aaron Leventhal]

Absolutely.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Yes!  In fact, if you want to do some fuzzing of accessibility code... that would be very nice.

Comment 21

[Aaron Leventhal]

We really need to torture test the accessibility code, and aren't doing well enough at it for 2 reasons:
2) Testing with screen readers on Windows doesn't get us crash reports because the COM actually swallows the crashes and just doesn't return from IAccessible2 method calls when there is a crash
2) Very few volunteer testers are running nightly builds on Linux with a11y

Jesse and Mats were doing some fuzz testing and about 6+ crashes were fixed as a result of that.

Comment 22

[Martijn Wargers (dead)]

Ok, note that there are still some crash bugs with testcases lying around:
https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=crash&product=Core&component=Disability+Access+APIs&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=testcase&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0=
I'm not complaining (fixing bugs is hard!), but it will be increasingly difficult to find new crash issues, when there are older ones hanging around.