Community
Participate
Working Groups
3.6RC1 Note that this behavior only happens when a screen reader is running. One of the new Accessibility API's we have provided in 3.6 is "what is the word at this offset?" I was surprised to see that when JAWS or AccProbe use this API, a word in the editor becomes selected. It happens every time the editor gets focus. I looked into it a bit and sent the following email to Felipe: "The strange selection behavior in getText(offset, WORD) is something that Eclipse is doing. For some reason, it thinks it got a double-click. This does not happen in CustomControlExample. I can show you the steps to see this using AccProbe inspector or JAWS. Here's a stack trace: Thread [main] (Suspended (breakpoint at line 9443 in StyledText)) StyledText.setSelection(int, int, boolean, boolean) line: 9443 StyledText.setSelectionRange(int, int) line: 9506 CompilationUnitEditor$AdaptedSourceViewer(TextViewer).setSelectedRange(int, int) line: 2391 JavadocDoubleClickStrategy(DefaultTextDoubleClickStrategy).doubleClicked(ITextViewer) line: 192 TextViewer$TextDoubleClickStrategyConnector.getPreviousOffset(MovementEvent) line: 245 StyledTextListener.handleEvent(Event) line: 90 EventTable.sendEvent(Event) line: 84 StyledText(Widget).sendEvent(Event) line: 1052 StyledText(Widget).sendEvent(int, Event, boolean) line: 1076 StyledText(Widget).sendEvent(int, Event) line: 1061 StyledText(Widget).notifyListeners(int, Event) line: 773 StyledText.sendWordBoundaryEvent(int, int, int, int, String, int) line: 8056 StyledText.getWordPrevious(int, int) line: 5378 StyledText$12.getText(AccessibleTextEvent) line: 6588 Accessible.get_textAtOffset(int, int, int, int, int) line: 4084 Accessible$9.method13(int[]) line: 459 COMObject.callback13(int[]) line: 182 OS.DispatchMessageW(MSG) line: not available [native method] OS.DispatchMessage(MSG) line: 2459 Display.readAndDispatch() line: 3655 Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2624 Workbench.runUI() line: 2588 Workbench.access$4(Workbench) line: 2422 Workbench$7.run() line: 670 Realm.runWithDefault(Realm, Runnable) line: 332 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 663 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149 IDEApplication.start(IApplicationContext) line: 115 EclipseAppHandle.run(Object) line: 196 EclipseAppLauncher.runApplication(Object) line: 110 EclipseAppLauncher.start(Object) line: 79 EclipseStarter.run(Object) line: 369 EclipseStarter.run(String[], Runnable) line: 179 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: not available DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: not available Method.invoke(Object, Object...) line: not available Main.invokeFramework(String[], URL[]) line: 619 Main.basicRun(String[]) line: 574 Main.run(String[]) line: 1407 Main.main(String[]) line: 1383 " Felipe replied: "This is in Text/JDT. They stop the majority of StyledText's actions (word next/word previous/ delete word previous/etc) and they do their own thing. (apparently) The only place where they let StyledText run is double click -> select word. They always select the word as a side effect of calling MovementListener#getPreviousOffset (during double click that doesn't matter cause StyledText would select the word anyway) We can disable and not call them during accessibility, but the word next that the accessible will see will not be the same as ctrl+arrow right key... " Would someone be able to look into this, please? It would be confusing to a blind user to use a keystroke to move forward a word, and then have a selected word as a side effect. If there is a simple fix, could it please be considered for RC2?
>They always select the word as a side effect of calling >MovementListener#getPreviousOffset "always" is not correct: we only select when the movement constant is set to MOVEMENT_WORD_START/END (START to be exact). So far this was the case during double-click only: Ctrl+Left/Right does not use those movement constants - it uses SWT.MOVEMENT_WORD instead. Unfortunately I cannot solve it on my side alone because I have no way to detect that I'm inside double-click+mouse-move: I get this sequence (this is Windows XP): mouseDown mouseUp getPrevious: false getNext: false mouseDown mouseDoubleClick mouseUp ==> during the first calls to getNext/Previous I don't know that it's happening during a double-click scenario. Either we use different movement constants for accessibility (or for double-click) or add an API field that allows me to detect whether we are in double-click mode. Maybe SWT could use the same movement constants that are used for Ctrl+Left/Right? Moving to SWT for further processing as there's nothing I can do on my side to fix this without breaking double-click selection.
Thank-you, Dani. We will have another look. Felipe, can you think of any way to do this using existing API?
(In reply to comment #1) > >MovementListener#getPreviousOffset > "always" is not correct: we only select when the movement constant is set to > MOVEMENT_WORD_START/END (START to be exact). Yes, and that alone is a bug in JFace/Text. The API is only requesting the offset of the previous word start, calling setSelection() on StyledText is unexpected. > So far this was the case during double-click only Correct, but you can't relie the API will only be called during double-click (the Javadoc does not say that). Right now we have another legit case where we need to call the API. > Unfortunately I cannot solve it on my side alone because > I have no way to detect that I'm inside double-click+mouse-move I don't think you need to know that. Do you ? You don't need to call setSelection() on StyledText. You just need to return the offsets correctly and StyledText itself will set the selection for you. > Either we use different movement constants for accessibility (or for > double-click) or add an API field that allows me to detect whether we are in > double-click mode. Maybe SWT could use the same movement constants that are > used for Ctrl+Left/Right? Is word start the only case you call setSelection() ? If that is so then we can replace MOVEMENT_WORD_START by MOVEMENT_WORD. On windows, MOVEMENT_WORD_START == MOVEMENT_WORD. > Moving to SWT for further processing as there's nothing I can do on my side to > fix this without breaking double-click selection. Ideally you should fix the bug, that is only way to avoid future problems. That said, I understand that this would be hard for you to change at this point. Moving back to Text. It is up to you if you will fix it or not. I'll let you know if we succeed working around this problem in SWT.
>Is word start the only case you call setSelection() ? Yes, but we have to make sure that the word end isn't sent either: they work together. I think the fix on your end would be to simply replace MOVEMENT_WORD_START and MOVEMENT_WORD_END with MOVEMENT_WORD in the accessibility case. Like it's already done when hitting Ctrl+Left/Right arrow. >The API is only requesting the offset of the previous word start, calling >setSelection() on StyledText is unexpected. Our double-click strategies select the text since day one and hence we could not change that without breaking all existing clients when we added the double-click + mouse-move word selection. Note that the whole movement event has only been added by SWT to support our scenario, see bug 138579 and that scenario was double-click + mouse-move selecting words. That we don't have constants for just that case is simply an oversight. Sorry, I'd really like to fix this for blind users but there is nothing I can do without help from SWT, hence moving back.
> Yes, but we have to make sure that the word end isn't sent either: they work > together. What does that mean to me ? select word on double click works as follow: start = getPreviousWordStart(offset) end = getNextWordEnd(start) where offset is where the double click occured. Do you call setSelection() on previous word start and on next word end ? If the only time you call setSelection() is during previous word start then I can workaround the problem on SWT. > I think the fix on your end would be to simply replace > MOVEMENT_WORD_START and MOVEMENT_WORD_END with MOVEMENT_WORD in the > accessibility case. Like it's already done when hitting Ctrl+Left/Right arrow. MOVEMENT_WORD can be word start or word end depending on the direction and on the platform. In accessibility we have to return exactly what they ask (and some place they ask specifically for word start and word end). > Our double-click strategies select the text since day one and hence we could > not change that without breaking all existing clients when we added the > double-click + mouse-move word selection. Note that the whole movement event > has only been added by SWT to support our scenario, Yes indeed, but when we added the movement event API we made it generic so it could be used in other cases. For example, we expected that using the API you would be able to implement camel case Ctrl+Left/Right arrow without stopping StyledText. > Sorry, I'd really like to fix this for blind users but there is nothing I can > do without help from SWT, hence moving back. I understand..
I read TextViewer#TextDoubleClickStrategyConnector() Besides calling setSelection() during previous word start, it also caches the selection in it (only clears on mouse up). The code expects previous word start/next word end only during mouse double click. Calling the API any other time can return wrong results. I see no option but not calling MovementListener for accessilibity. Silenio ? Dani, FYI, StyledText calls MovementListener is other cases besides double click, for example, it is called for double-click-drag and some other cases for block selection.
Created attachment 169414 [details] patch Car, can you test this patch. Note that without using Text our word next/previous won't be camel case.
> for example, it is called for double-click-drag Well, if you read bug 138579 again, then you'd know that we explicitly added the code to support exactly that scenario ;-). The fix will work but it is not the right thing to do in my opinion. A better fix is to introduce two new movement constants that are used only when double-click or double-click-drag happens, e.g. MOVEMENT_DOUBLE_CLICK_WORD_START/END. The code in StyledText needs to get changed to also send those events (in addition in order not to break existing clients) and Text would switch to use those new constants in its TextDoubleClickStrategyConnector. I would PMC+ this API addition and suspect that the code changes are even smaller than in the suggested patch. The Javadoc of the new constant must mention that MOVEMENT_WORD_START/END are also sent in case of double-click and double-click-drag but that the new constants should when a client only wants to deal with the double-click and double-click-drag cases. A similar comment should be added to MOVEMENT_WORD_START/END.
Right now TextViewer#TextDoubleClickStrategyConnector only handles SWT.MOVEMENT_WORD_START during getPreviousOffset() and SWT.MOVEMENT_WORD_END during getNextOffset(). My understand is that you would change TextDoubleClickStrategyConnector to replace: SWT.MOVEMENT_WORD_START by MOVEMENT_DOUBLE_CLICK_WORD_START, and SWT.MOVEMENT_WORD_END by MOVEMENT_DOUBLE_CLICK_WORD_END. But will you add new code in TextDoubleClickStrategyConnector to handle SWT.MOVEMENT_WORD_START and SWT.MOVEMENT_WORD_END without selecting text ? I think I rather add a detail bit to the event indicating that SWT.MOVEMENT_WORD_START/END is a mouse double click/mouse drag context. The constants SWT.MOVEMENT_DOUBLE_CLICK_WORD_START/END would not make sense for TextLayout.
>But will you add new code in TextDoubleClickStrategyConnector to handle >SWT.MOVEMENT_WORD_START and SWT.MOVEMENT_WORD_END without selecting text ? No. >I think I rather add a detail bit to the event indicating that >SWT.MOVEMENT_WORD_START/END is a mouse double click/mouse drag context. That's also be perfectly fine by me, see comment 1: >or add an API field that allows me to detect whether we are in >double-click mode.
(In reply to comment #10) > >But will you add new code in TextDoubleClickStrategyConnector to handle > >SWT.MOVEMENT_WORD_START and SWT.MOVEMENT_WORD_END without selecting text ? > No. Since you will not handle these constants, calling the listener or not calling the listener will yield the same result: no camel case for accessibility. is that right, or am I missing something here ?
> Since you will not handle these constants, calling the listener or not calling > the listener will yield the same result: no camel case for accessibility. > > is that right, or am I missing something here ? When using Ctrl+Left/Right it should go through our actions. However, reading the word would ignore camel-case boundaries.
That might be ok, but it sounds a little funny under a screen reader. Say you have the following character sequence, with the caret at |: | getNextWord(anArg); Now, say the user types ctrl+right arrow. The cursor goes to: get|NextWord(anArg); And the screen reader says "getNextWord". If the user types ctrl+right arrow again, the cursor goes to: getNext|Word(anArg); And the screen reader says "getNextWord" again. And if the user types ctrl+right arrow a third time, the cursor goes to: getNextWord|(anArg); And the screen reader says "getNextWord" for the third time. We don't think it sounds right to have the word spoken 3 times as the user moves through the word. I am going to see if I can find out what the "correct" way to handle camel case navigation under a screen reader is. I will get back to you. For now, however, the "unexpected selection" is important enough to warrant a new detail bit (in my opinion <grin>). It is important, because it also caches. I had a very strange case where the unexpected selection happened between a pair of round brackets. From then on, the screen reader spoke the entire (...) no matter what navigation I did inside the brackets. It was completely misleading.
>For now, however, the "unexpected selection" is important enough to >warrant a new detail bit (in my opinion <grin>) I agree. We should do this for RC3 (not causing RC2 rebuild). Fixing the camel-case issue would be very hard - if not impossible - to solve because the navigation strategies are provided by the editors and unless they (read: all) implement a new yet to be defined interface it won't work. I think the best we can do for the editor we control (Java editor in this case) is to add a section to our Accessibility doc which suggests to disable camel-case navigation: Java > Editor: [ ] Smart caret positioning in Java names
Yes, a user option to disable camel case navigation might be the best way to handle the camel case problem.
>Yes, a user option to disable camel case navigation might be the best way to >handle the camel case problem. And the good news is, that this option is already there :-) Filed bug 314010 for the documentation work.
The unexpected selection problem is not fixed by disabling the camel case. To fix this problem we have two options: 1) Not calling the listeners 2) Adding new API At this point we prefer option (1). The change in the code is very simple and safe; only involves changes in only one component (SWT); doesn't require special approval (API does); the code was tested and verifed to work; the final behaviour of both solutions is the same. After 3.6 ships we can think about an API. Dani, what do you think ?
>Dani, what do you think ? I'm fine with 1) as long as Carolyn confirms it's working.
+1 for RC3.
Yes - thanks, Dani. The patch in comment 7, plus deselecting "Java > Editor: [ ] Smart caret positioning in Java names" gives a good user experience for screen readers. I tried it with 2 different screen readers, and the strange selection behavior is gone. +1 for RC3.
Fixed in HEAD > 2010-05-26
.
Please open a new bug for the API.
