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 578602 - atk_object_add_relationship doesn't add relationship which already exists, but returns TRUE
atk_object_add_relationship doesn't add relationship which already exists, bu...
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: atk
1.25.x
Other Linux
: Normal normal
: ---
Assigned To: Li Yuan
Li Yuan
Depends on:
Blocks:
 
 
Reported: 2009-04-10 13:54 UTC by Andrey Tsyvarev
Modified: 2012-03-26 18:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
reproduce bug (1.31 KB, text/plain)
2009-04-13 08:10 UTC, Andrey Tsyvarev
  Details
Fixes the bug (917 bytes, patch)
2011-01-02 00:47 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
accepted-commit_now Details | Review

Description Andrey Tsyvarev 2009-04-10 13:54:04 UTC
From description of function

gboolean atk_object_add_relationship(AtkObject *object, AtkRelationType relationship, AtkObject *target):

Adds a relationship of the specified type with the specified target.
...
Returns : TRUE if the relationship is added.

After patch at 2008-11-10, which fix bug 477708, the function doesn't add relationship, if it already exists in object's relation set. But it returns TRUE anyway, that's wrong.
Comment 1 Andrey Tsyvarev 2009-04-13 08:10:50 UTC
Created attachment 132581 [details]
reproduce bug

On atk-1.25 and later test output is:

Number of relationships after first call: 1.
Second call of atk_object_add_relationship returns TRUE.
Number of relationships after second call: 1.
Comment 2 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-01-02 00:47:22 UTC
Created attachment 177335 [details] [review]
Fixes the bug

The straightforward solution for this bug is just return FALSE is the relation is already present on the object relation set.
Comment 3 Li Yuan 2011-01-05 05:43:39 UTC
Review of attachment 177335 [details] [review]:

Patch looks good.
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-01-05 11:41:04 UTC
(In reply to comment #3)
> Review of attachment 177335 [details] [review]:
> 
> Patch looks good.

Ok, I will apply it during the day.
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-01-05 11:48:43 UTC
Patch applied, so bug fixed. But I can't close this bug as FIXED.

Li or Andrey, could you close the bug? Thanks
Comment 6 Stew Benedict 2012-03-26 15:43:53 UTC
With the commit of the fix, I'm seeing a different set of failures out of lsb-test-desktop-t2c (which I believe the original reported developed). It would appear than now trying to add a different target with a type already used to a relation set also fails. Is this the desired behavior?
Comment 7 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-03-26 18:18:25 UTC
(In reply to comment #6)
> With the commit of the fix, I'm seeing a different set of failures out of
> lsb-test-desktop-t2c (which I believe the original reported developed). It
> would appear than now trying to add a different target with a type already used
> to a relation set also fails. Is this the desired behavior?

After a look to the code I think that you are right.

Before my patch (see comment 2), if the target was not included on the relationship, it gets the relation and adds the target. So in order to check if the relationship was added or not, you need to check the relationship and the target. Something that current relationset API doesn't have. So a new method is required:

/**
 * atk_relation_set_contains_extra:
 * @set: an #AtkRelationSet
 * @relationship: an #AtkRelationType
 * @target: an #AtkObject
 *
 * Determines whether the relation set contains a relation that matches the
 * specified type and target
 *
 * Returns: %TRUE if @relationship is the relationship type of a relation
 * in @set with @target as one of his targets, %FALSE otherwise

And replace atk_relation_set_contains with atk_relation_set_contains_extra (this name is temporal, a real one is required).

Anyway, today is GNOME 3.4 release day, so I can include it. I will work on that for the next release 3.6

Meanwhile, I will create a bug to handle the bug introduced and the new API required.
Comment 8 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-03-26 18:24:39 UTC
(In reply to comment #7)

> Meanwhile, I will create a bug to handle the bug introduced and the new API
> required.

Done, bug 672869.