Bug 236735 - [Forms] FormText fields not accessible
Summary: [Forms] FormText fields not accessible
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: User Assistance (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: 3.5 M1   Edit
Assignee: Adam Archer CLA
QA Contact:
URL:
Whiteboard:
Keywords: accessibility
Depends on:
Blocks:
 
Reported: 2008-06-11 16:31 EDT by Mike Higginbotham CLA
Modified: 2008-07-08 15:33 EDT (History)
5 users (show)

See Also:


Attachments
Snippet demonstrating caret display bug (2.28 KB, text/plain)
2008-06-11 18:02 EDT, Mike Higginbotham CLA
no flags Details
patch for 3.4 to rollback fix for bug 178557 (1.57 KB, patch)
2008-06-17 18:04 EDT, Adam Archer CLA
no flags Details | Diff
patch to rollback bug 178557 and add NO_FOCUS support (2.60 KB, patch)
2008-06-17 18:12 EDT, Adam Archer CLA
no flags Details | Diff
patch for 3.5 M1 (4.43 KB, patch)
2008-07-02 17:53 EDT, Adam Archer CLA
no flags Details | Diff
patch to add focus box (3.90 KB, patch)
2008-07-08 15:27 EDT, Adam Archer CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Higginbotham CLA 2008-06-11 16:31:35 EDT
Build ID: I20080530-1730

Steps To Reproduce:
1. Use a FormText widget on a form.
2. Try to tab to it. Unless there is a link in the FormText, it will not get focus and screen readers, e.g., Vista Narrator, will not read it.

More information:
This was introduced by the fix to 178557. Prior to that fix, FormText fields received focus but did not indicate such (they did not display a caret). After that fix, when focus is given to a FormText it passes the focus on if it does not have any selectable elements, e.g., a link. As far as I know, this makes them practically invisible to screen readers like Vista Narrator.
Comment 1 Adam Archer CLA 2008-06-11 16:58:33 EDT
To address this while still allowing the new behaviour as discussed in bug 178557, we will revert to the old behaviour by default (allow tabstops), but will support the use of the SWT.NO_FOCUS style on FormText, similarly to how the SWT Text widget works.
Comment 2 Chris Goldthorpe CLA 2008-06-11 17:16:43 EDT
Hi Adam,

I'm wondering if it is appropriate to change the behavior in this way in 3.4.1, supporting a new style bit feels like an API change. Copying McQ for his opinion.
Comment 3 Mike Wilson CLA 2008-06-11 17:50:01 EDT
Shouldn't text fields in forms should behave exactly like Text widgets in normal Eclipse layouts? Why is extra API needed?

Comment 4 Adam Archer CLA 2008-06-11 17:59:01 EDT
FormText is not a text field. It is used to display styled, wrapping text with embedded links and controls (essentially a lightweight, simplified browser).

When it receives focus it gives focus to its first focusable child (a link or control). If it has none, it now passes focus on to its next sibling (as of bug 178557), while it used to simply accept focus and do nothing with it. This new behaviour is preventing the content from being read by screen readers which is why Mike is requesting a roll back of the fix. His product depends on FormText having a tab stop and, therefore, being read by screen readers.
Comment 5 Mike Higginbotham CLA 2008-06-11 18:02:39 EDT
Created attachment 104566 [details]
Snippet demonstrating caret display bug

The solution to 178557 introduced/exposed a display bug. The problem in 178557 was that the FormText field had focus but there was no visual evidence. We had gotten around that by adding a Caret to the widget using FormText.setCaret. There was now visual evidence that the FormText had focus. The caret did not have all the expected caret behavior (see 236739) but was a sufficient work around. After the change in 178557, our FormText widgets no longer received focus but still had their carets. In addition to the fields no longer being read by screen readers, this resulted in the first focusable field after the FormText having focus but the caret for that field being displayed in the FormText field. The attached snippet demonstrates this. If you forward tab, the caret is displayed incorrectly. However, if you back tab everything displays fine.
Comment 6 Mike Wilson CLA 2008-06-12 08:52:27 EDT
Adding Steve N since he's likely a better choice for reviewer. Steve, note that this is for 3.4.1.
Comment 7 Steve Northover CLA 2008-06-12 12:59:59 EDT
For 3.4.1, are you going to support SWT.NO_FOCUS in FormText?

Felipe, please investigate the snippet to see whether this is a separate SWT bug (ie. "If you forward tab, the caret is displayed incorrectly").
Comment 8 Adam Archer CLA 2008-06-12 13:17:59 EDT
Yes, my plan was to add support for SWT.NO_FOCUS in 3.4.1. I do not want to rollback the change without adding support for the flag as it would re-expose bug 178557 with no workaround. If we cannot add the flag in 3.4.1, then I propose that we defer the entire fix to 3.5. If we do this, I can provide a patch that Mike can use in the meantime to solve his problem.
Comment 9 Adam Archer CLA 2008-06-13 15:35:36 EDT
I just realized what I said in comment 1 was incorrect. SWT.NO_FOCUS is actually not a valid flag for the Text widget, but this is because it is not possible to enforce with the native widget on all platforms, so this should not stop us from using this approach for FormText.

http://dev.eclipse.org/newslists/news.eclipse.tools/msg24829.html
Comment 10 Adam Archer CLA 2008-06-17 17:56:58 EDT
I have investigated this issue a little. Adding the NO_FOCUS support is a little more complicated than I had originally anticipated. I do not want to do it for 3.4.1. I also do not want to roll back the fix for bug 178557 without providing a workaround. For that reason, I am deferring the fix to 3.5.

I will provide a patch for the 3.4 code base that rolls back the fix in bug 178557. This can be used as a temporary solution until the permanent fix is available in 3.5.
Comment 11 Adam Archer CLA 2008-06-17 18:04:09 EDT
Created attachment 105216 [details]
patch for 3.4 to rollback fix for bug 178557

This patch can be applied to the 3.4 code to roll back the fix for bug 178557. As discussed above, it will not make it into 3.4.1.
Comment 12 Adam Archer CLA 2008-06-17 18:12:06 EDT
Created attachment 105217 [details]
patch to rollback bug 178557 and add NO_FOCUS support

This is a potential fix for 3.5 that rolls back the fix for bug 178557 and also adds FormText support for SWT.NO_FOCUS. It needs to be more thoroughly tested, but from preliminary testing, it seems to be okay.

One problem with this approach is that FormToolkit does not have a createFormText methods that accept style parameters (until now it did not support any style bits). This means that to consume this change clients will have to create their FormTexts manually and use the toolkit to adapt them. As an alternative we could add a new createFormText method that does accept style bits. However, I would like to avoid adding API if possible, so I want to investigate a couple methods of painting focus on FormText rather than adding support to prevent focus. This would be an alternative solution to bug 178557.

Potential ideas for painting focus are:
-Add a carat (bug 236739)
-Paint a focus box around the entire FormText when there are no focusable children
Comment 13 Felipe Heidrich CLA 2008-06-18 14:20:21 EDT
I opened Bug 237675 to capture the problem described in comment #5


Personall note: If you want your control to indicate when it has focus why didn't you use GC#drawFocus instead of a Caret ?
Comment 14 Adam Archer CLA 2008-06-19 16:37:19 EDT
Need to ensure that the 3.5 fix also corrects bug 213732.
Comment 15 Adam Archer CLA 2008-07-02 17:53:50 EDT
Created attachment 106362 [details]
patch for 3.5 M1

I investigated the possibility of adding a focus box or a caret to FormText. Both proved non-trivial. Also, they change the current functionality of the widget which would likely provoke complaints.

For this reason, I've gone with support for SWT.NO_FOCUS for 3.5M1. FormText will once again receive focus by default (and will not paint anything to indicate it has it). To address the issue of accepting focus without painting it I will consider adding a FormText.PAINT_FOCUS style bit (or something similar) which will cause FormText to paint its focus. This work will be tracked in bug 236739.

Attached is a new patch that also adds an API method to FormToolkit to create a FormText with the given style bits. Currently SWT.NO_FOCUS will be the only supported style.
Comment 16 Adam Archer CLA 2008-07-02 17:55:12 EDT
Patch applied to HEAD.
Comment 17 Steve Northover CLA 2008-07-02 18:06:11 EDT
I wish I understood better exactly what you were trying to do.  Looking at the patch code only, overriding forceFocus() and tracking focus yourself seems bad.  Also, did you introduce new API in an x.x.x release?

Felipe and I recreated the bug in Eclipse but couldn't quite follow what was going on.  Do you have an SWT only snippet that shows what you are trying to do?
Comment 18 Adam Archer CLA 2008-07-03 11:21:47 EDT
We did not add API in an x.x.x release. The latest patch was applied to HEAD for 3.5 (o.e.ui.forms 3.4.0).

All we are trying to do is provide a mechanism by which the FormText can be made non-focusable. For 3.4, this was done by default when FormText had no focusable children (links, buttons, etc.), but that caused an accessibility problem for some products (see above).

To solve the accessibility problem, we are reverting to the previous behaviour and adding support for the NO_FOCUS flag so clients can still achieve the 3.4 functionality. The only reason that forceFocus has been overridden is so that focus can be prevented when the NO_FOCUS flag is set. FormText was always tracking its own internal focus, so nothing has changed there.
Comment 19 Adam Archer CLA 2008-07-08 12:02:49 EDT
The NO_FOCUS solution to this problem is not ideal. It requires consumers to use a style bit to work around a bug (not painting focus) and using the style bit exposes an accessibility bug. It forces consumers to trade one bug for another. Instead we will simply paint a focus box around the widget in this case. This solves the original problem reported in bug 178557 and does not cause any accessibility problems or changes in behaviour of the widget.
Comment 20 Adam Archer CLA 2008-07-08 15:27:46 EDT
Created attachment 106873 [details]
patch to add focus box

This patch removes the NO_FOCUS support added in the previous patch and adds a focus box to FormText when it does not have focusable children.
Comment 21 Adam Archer CLA 2008-07-08 15:33:18 EDT
Patch applied to HEAD.