Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 312795 - Accessible Relations should only be added once
Summary: Accessible Relations should only be added once
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 RC1   Edit
Assignee: Carolyn MacLeod CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-13 10:21 EDT by Carolyn MacLeod CLA
Modified: 2010-05-13 12:59 EDT (History)
1 user (show)

See Also:
carolynmacleod4: review+
Silenio_Quarti: review+

Attachments
patch checks containsTarget before adding relation (1.68 KB, patch)
2010-05-13 10:21 EDT, Carolyn MacLeod CLA 		no flags Details | Diff
this is the correct patch (1.27 KB, patch)
2010-05-13 10:32 EDT, Carolyn MacLeod CLA 		no flags Details | Diff
patch for GTK (2.01 KB, patch)
2010-05-13 11:25 EDT, Silenio Quarti CLA 		no flags Details | Diff
patch for cocoa (1.24 KB, patch)
2010-05-13 11:49 EDT, Carolyn MacLeod CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Carolyn MacLeod CLA 2010-05-13 10:21:55 EDT 
Created attachment 168384 [details]
patch checks containsTarget before adding relation

Before adding a new relation in Accessible.addRelation, we need to check that the relation is only being added once. This came up because the first thing the UI team did was to add a Relation in response to a UI event, and the same relation was being added over and over. For context, here is the relevant piece of the UI patch:

+++ src/org/eclipse/jface/dialogs/TitleAreaDialog.java
 	private void updateMessage(String newMessage) {
+		Display display = workArea.getDisplay();
+		Control focusControl = display.getFocusControl();
+		if (focusControl != null)
+			focusControl.getAccessible().addRelation(ACC.RELATION_DESCRIBED_BY, messageLabel.getAccessible());
 		messageLabel.setText(newMessage);
+		if (focusControl != null)
+			focusControl.getAccessible().sendEvent(ACC.EVENT_DESCRIPTION_CHANGED, null);
 	}

The attached patch fixes this by checking first whether the target was already added for the relation type.
Comment 1 Carolyn MacLeod CLA 2010-05-13 10:32:20 EDT 
Created attachment 168387 [details]
this is the correct patch

I attached the wrong patch previously. This one has only the fix mentioned.
Comment 2 Silenio Quarti CLA 2010-05-13 11:25:45 EDT 
Created attachment 168396 [details]
patch for GTK
Comment 3 Carolyn MacLeod CLA 2010-05-13 11:49:13 EDT 
Created attachment 168402 [details]
patch for cocoa
Comment 4 Silenio Quarti CLA 2010-05-13 11:51:29 EDT 
Car, please review the gtk patch
Comment 5 Carolyn MacLeod CLA 2010-05-13 12:37:43 EDT 
The gtk patch looks ok - I think you should probably implement hashCode as well, though. Maybe something like:
		public int hashCode () {
			return target.hashCode() ^ type;
		}

I need to test on your machine.
Comment 6 Silenio Quarti CLA 2010-05-13 12:49:26 EDT 
Relation is not been added to hash tables right now. I will add hashCode() if I ever add a relation to a set/hash table.
Comment 7 Carolyn MacLeod CLA 2010-05-13 12:56:06 EDT 
Windows and Mac patch are committed.
Comment 8 Carolyn MacLeod CLA 2010-05-13 12:59:31 EDT 
Fixed > 20100513