After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 480501 - readPageSummary() prevents access to FF status bar contents
readPageSummary() prevents access to FF status bar contents
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: speech
2.20.x
Other All
: Normal normal
: 2.20.1
Assigned To: Joanmarie Diggs (IRC: joanie)
Orca Maintainers
Depends on:
Blocks: 404403
 
 
Reported: 2007-09-26 08:04 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2007-10-30 23:01 UTC
See Also:
GNOME target: ---
GNOME version: 2.19/2.20


Attachments
proposed patch (638 bytes, patch)
2007-09-26 08:06 UTC, Joanmarie Diggs (IRC: joanie)
reviewed Details | Review
take 2 (2.27 KB, patch)
2007-10-11 00:30 UTC, Joanmarie Diggs (IRC: joanie)
needs-work Details | Review
take 3 (2.34 KB, patch)
2007-10-11 17:11 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
take 4 (3.81 KB, patch)
2007-10-11 22:04 UTC, Joanmarie Diggs (IRC: joanie)
committed Details | Review

Description Joanmarie Diggs (IRC: joanie) 2007-09-26 08:04:14 UTC
According to the Orca spec:

* Double-clicking the "Where am I" key in Firefox provides a page 
  summary, which includes the total number of headings, forms, 
  tables, visited links, and unvisited links in the document.

Also according to the Orca spec:

* Detailed information: Double Press of KP_Enter
* Status bar: Double Press of Insert+KP_Enter

I assume that means the page summary should take place for an unmodified double press of KP_Enter and that for a modified double press of KP_Enter, Orca should read the Firefox status bar and not do a page summary.  Currently the page summary kicks in for both unmodified and modified double presses of KP_Enter.
Comment 1 Joanmarie Diggs (IRC: joanie) 2007-09-26 08:06:15 UTC
Created attachment 96211 [details] [review]
proposed patch

Mike please test.
Comment 2 Mike Pedersen 2007-10-01 17:15:06 UTC
Orca does now seem to work more correctly with this patch.  In the process of testing this patch I believe I've found a usability problem.  
Currently on a page such as google or bugzilla if you press the "read status bar" command you are also given the default button.  I personally think this is a bad idea because if the user presses enter when focus is on a certain link, that link will be activated not the default button.  Perhaps if we can tell we are in a dialog box, we should restrict the default button reading to dialog boxes only.  
Comment 3 Joanmarie Diggs (IRC: joanie) 2007-10-01 17:25:07 UTC
> Perhaps if we can tell we
> are in a dialog box, we should restrict the default button reading to dialog
> boxes only.  

We can't. <smile>  That was the original plan, but it turns out that more often than not the "dialogs" for which we want default buttons aren't actually dialogs.  We have an open bug against Gail (bug #468122) requesting that they expose window type hints to us so that we can better identify dialogs that aren't dialogs but are dialogs. 

So in the meantime we can:

1. Do as you suggest, with the side effect being we will not announce a number of default buttons for functional-but-not-actual dialogs until the Gail bug is fixed and we implement support on our end to actually look for those hints.

2. Continue to check all windows for the presence of default buttons, but special-handle Gecko page content to avoid the situation you describe.

Thoughts?

Comment 4 Mike Pedersen 2007-10-01 17:33:02 UTC
> 2. Continue to check all windows for the presence of default buttons, but
> special-handle Gecko page content to avoid the situation you describe.
> 
> Thoughts?
I personally think option 2 is a much better choice. 
Comment 5 Willie Walker 2007-10-01 20:53:57 UTC
(In reply to comment #2)
> Orca does now seem to work more correctly with this patch.  In the process of
> testing this patch I believe I've found a usability problem.  
> Currently on a page such as google or bugzilla if you press the "read status
> bar" command you are also given the default button.  I personally think this is
> a bad idea because if the user presses enter when focus is on a certain link,
> that link will be activated not the default button.  Perhaps if we can tell we
> are in a dialog box, we should restrict the default button reading to dialog
> boxes only.  

I think pressing return in a single line text area will have the effect of invoking the default button.  If this is the case, it seems as though it might be important to present the default button as part of Where Am I.
Comment 6 Willie Walker 2007-10-01 20:57:09 UTC
Oops -- sorry.  I was trying to verify this.  I pressed return while in the "Alias" field and sure enough, the page was committed.
Comment 7 Joanmarie Diggs (IRC: joanie) 2007-10-01 23:34:02 UTC
> I think pressing return in a single line text area will have the effect of
> invoking the default button.  If this is the case, it seems as though it might
> be important to present the default button as part of Where Am I.

Okay, revised "in the meantime":

1. Leave the patch as it is

2. Check to see if we're in web content but NOT in a single line text area. 

Comment 8 Willie Walker 2007-10-02 00:34:26 UTC
(In reply to comment #7)
> > I think pressing return in a single line text area will have the effect of
> > invoking the default button.  If this is the case, it seems as though it might
> > be important to present the default button as part of Where Am I.
> 
> Okay, revised "in the meantime":
> 
> 1. Leave the patch as it is
> 
> 2. Check to see if we're in web content but NOT in a single line text area. 

Or maybe even in plain old text.  I'm going to try putting the focus on the "Leave as Assigned" radio button and press return.  If you see this message, it's because the form was submitted when I did that.  If that works, I suspect other things may crop up as well.  So, perhaps maybe the best thing is to leave the patch as is, with the understanding that the user should infer what will happen when they press return, which I suspect is the same thing that a sighted user needs to infer.
Comment 9 Mike Pedersen 2007-10-03 03:10:48 UTC
In the orca team meeting today we decided the following.  
The patch should be changed so that when on a standard web page orca should not speak the default button.  The default button should still be spoken in xsul or aria dialogs.
Comment 10 Joanmarie Diggs (IRC: joanie) 2007-10-10 21:10:10 UTC
Hmmmm.... Gecko.py has all the smarts for making the decision Mike specified.  where_am_I.py doesn't have any public (I assume that's what they're called) methods.  It seems to me are choices are:

1. Give where_am_I.py Gecko smarts.  (Personally this seems like a bad idea, but I'm the new kid)

2. Create a new argument for whereAmI(), say, speakDefaultButton=True and then set it to False from Gecko.py.

3. Create a new method in where_am_i.py like speakStatusBar().

I'm not sure if #2 or #3 would be better.  Or if there's a more clever way to accomplish what we want.  Will?
Comment 11 Willie Walker 2007-10-10 21:24:07 UTC
> 3. Create a new method in where_am_i.py like speakStatusBar().

Subclasses should be able to override superclass methods that begin with _.  That is, I believe the _ prefix is a convention to indicate the method is not for public use, but can be used/overridden by subclasses.

So, I think Gecko's WhereAmI subclass can just override _speakStatusBar().  :-)

Comment 12 Joanmarie Diggs (IRC: joanie) 2007-10-10 21:27:11 UTC
Awesome.  I can totally do that.  Thanks!!
Comment 13 Joanmarie Diggs (IRC: joanie) 2007-10-11 00:30:53 UTC
Created attachment 97039 [details] [review]
take 2

When I thought about it, what we really seemed to need is our own _getDefaultButton() so I overrode that instead. :-)  I also added a new argument to it, checkLocation=True.  The reasoning is this:  _getDefaultButton() is recursive and a bit on the brute-force-ish side.  If we have concluded that we're drilling down, we don't want to be calling inDocumentContent() and isAriaWidget() for each and every object when we know those tests will fail anyway.

Beyond this you will notice a change to inDocumentContent().  When I tried the patch on the dojo dialogs (http://archive.dojotoolkit.org/dojo-2007-09-20/dojotoolkit/dijit/tests/test_Dialog.html), I noticed that we were failing the test for dialogishness.  It seems that dojo dialogs are of ROLE_DIALOG, but ulimately we find an ancestor of ROLE_DOCUMENT_FRAME and conclude that we're in web content.  These objects are also failing the isAriaWidget() test, so it seemed some other check was in order.  This one seems safe I think.  

NOTE:  In the process I believe I found a bug in dojo or Firefox or both.  This patch works with the "Show Tooltip Dialog" but fails with the other two I tried:  Show Dialog and Programatic Dialog (3 second delay).  When I examined the hierarchy of those two, there was an object of ROLE_DIALOG, but the only child seemed to be a single text object which represented the "x" to close the "window".  The dialog had a sibling of ROLE_SECTION which contained the objects serving as the dialog's controls.  That certainly makes it hard to identify if we're in a dojo dialog....

UPDATE:  I just grabbed the very latest Firefox to be sure that what I just said was still the case.  It's not; now it's worse:  We don't seem to have any dojo objects of ROLE_DIALOG the things that did have ROLE_DIALOG are now of ROLE_SECTION. :-(  I looked at the dojo archive from 10/10 and it's true there as well.

Anyhoo....

Will, did I get it right from the Python point of view, and do you think checking for ROLE_DIALOG in inDocumentContent() is a reasonable thing to do? OR should we take it out and wait and see how the dojo stuff pans out?
Comment 14 Willie Walker 2007-10-11 14:57:30 UTC
Thanks Joanie!

I think _getDefaultButton should always return the default button if one exists. That is, I think the decision to not speak the default button should not be in _getDefaultButton, but should be elsewhere (e.g., _speakDefaultButton).

To avoid the brute forcish thing, instead of where_am_i setting a self._defaultButton, maybe self._speakDefaultButton could just call self._getDefaultButton?

> Will, did I get it right from the Python point of view, and do you think
> checking for ROLE_DIALOG in inDocumentContent() is a reasonable thing to do? OR
> should we take it out and wait and see how the dojo stuff pans out?

I think the ROLE_DIALOG thing may be something that should be handled in a different bug.  :-)
Comment 15 Joanmarie Diggs (IRC: joanie) 2007-10-11 17:11:47 UTC
Created attachment 97070 [details] [review]
take 3

How 'bout this? :-)
Comment 16 Willie Walker 2007-10-11 17:28:37 UTC
(In reply to comment #15)
> Created an attachment (id=97070) [edit]
> take 3
> 
> How 'bout this? :-)
> 

It seems like it might work.  I find the whole self._defaultButton field confusing though.  It looks like it was there as a performance enhancement (e.g., we find it just once and never look again), but I'm not sure that performance enhancement was ever used.  Instead, it looks like it was just 'drugs' instead of 'performance enhancing drugs'.  I'd opt for just getting rid of it altogether if this makes sense (and I'm not the one on drugs).
Comment 17 Joanmarie Diggs (IRC: joanie) 2007-10-11 17:56:22 UTC
I thought it was in there to prevent us from further descending the hierarchy unnecessarily once we've found the statusBar or -- in this case -- defaultButton. I'll take it out.
Comment 18 Joanmarie Diggs (IRC: joanie) 2007-10-11 22:04:34 UTC
Created attachment 97093 [details] [review]
take 4

Okay that was easier said than done. :-)  But I *think* it's done.

Thank you Rich for helping me with this! (And I gather enlisting Will, so thank you Will).
Comment 19 Willie Walker 2007-10-11 23:33:55 UTC
> Okay that was easier said than done. :-)  But I *think* it's done.

Recursion reschmursion.  Looks good to me!  Please commit.  Is this also something good for gnome2-20?  :-)
Comment 20 Joanmarie Diggs (IRC: joanie) 2007-10-12 00:04:21 UTC
> Recursion reschmursion.  

Or as Barbie might say before she got PC:  "Recursion is hard, let's go shopping!"

> Looks good to me!  Please commit.  

Done.

> Is this also something good for gnome2-20?  :-)

Yeah, probably so.  And safe too.  Done.

Thanks again guys!!

Comment 21 Mike Pedersen 2007-10-30 18:00:06 UTC
This one looks good to me. 
Comment 22 Joanmarie Diggs (IRC: joanie) 2007-10-30 23:01:59 UTC
Thanks.  Closing as FIXED.