| Summary: | [FieldAssist] ContentAssistCommandAdapter enablement not toggled when field has focus | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||||
| Component: | UI | Assignee: | Susan McCourt <susan> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | ob1.eclipse, pwebster, remy.suen, susan | ||||||
| Version: | 3.6 | Flags: | pwebster:
review+
ob1.eclipse: review+ |
||||||
| Target Milestone: | 3.6 RC1 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Markus Keller
(In reply to bug 301196 comment #20) I would replace the "CONTROL_ID_PREFIX + hashCode()" hack by "CONTROL_ID_PREFIX + nextID++" (and a new "private static int nextID = 0;") Markus, would you mind running TextContentAssistCommandAdapterTest.testBug301196CorrectHandlerConflictResolution() on the Mac to see if it would have caught this failure? You have to re-enable the tests in the ui FieldAssistTestSuite in order to run them (the test suite has these tests diabled in the builds for other reasons). If the error is caught by the test, (and corrected by the fix), then I will investigate separating this test out to ensure it runs in the build (and on all platforms). (In reply to comment #2) Nope, testBug301196CorrectHandlerConflictResolution() and testHandlerPromptsPopup() are still green. The last 3 tests fail with: junit.framework.AssertionFailedError: unable to post event to display queue for test case at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.assertTrue(Assert.java:20) at org.eclipse.jface.tests.fieldassist.AbstractFieldAssistTestCase.sendKeyDownToControl(AbstractFieldAssistTestCase.java:168) Assigning to M7, there's no reason to introduce a regression and the fix is safe. Will attach a patch with the proposed fix. Oleg, I will need help verifying this on a Mac (I trust Markus already did this but I'd feel better with a real patch and verification.) moving back to RC1, Markus says this is not urgent. Created attachment 166814 [details]
patch
This patch ties adapter enablement to handler enablement rather than including it in the activation expression. The handler is made a non-anonymous class so that we can expose the enabling protocol.
Paul, can you please review this patch? Oleg, can you please verify that this patch actually fixes the observed problem on the Mac? I can only verify that it still works on Windows. ;-) (copied from bug 199127) --------------------------- To reproduce, open the Find/Replace dialog on a text editor and then toggle the Regular Expressions checkbox. The lightbulb on the find field is correctly added/removed, but the availability of Ctrl+Space to open the content assist proposals popup is only updated when the field has lost focus. The problem is not reproducible on Windows, probably because the find field always loses focus on Windows when the checkbox is toggled. Created attachment 166818 [details]
the correct patch
this is the real patch. The other one is from an old bug.
(In reply to comment #8) > Oleg, > can you please verify that this patch actually fixes the observed problem on > the Mac? works for me. Fixed in HEAD >20100503. Markus, can you please verify in the next build? Verified in HEAD. |