There's another difference afaik, and it's that with the "property-change" signal you can report both the old and new values for the property that has changed (through the AtkPropertyValues struct), although documentation says that is barely used, only with detail "accessible-value":

http://library.gnome.org/devel/atk/unstable/AtkObject.html#AtkPropertyValues

So, I'm not sure about deprecating this one, although I agree it's at least confusing to have both the "notify" and the "property-change" signals.

(In reply to comment #1)
> There's another difference afaik, and it's that with the "property-change"
> signal you can report both the old and new values for the property that has
> changed (through the AtkPropertyValues struct), although documentation says
> that is barely used, only with detail "accessible-value":
> 
> http://library.gnome.org/devel/atk/unstable/AtkObject.html#AtkPropertyValues

Anyway, as we already discussed in person, this old_value is not really used at this moment, as most of the objects on gail basically cached old values.

> So, I'm not sure about deprecating this one, although I agree it's at least
> confusing to have both the "notify" and the "property-change" signals.

This is the reason I opened the bug ;)

If finally this signal is not deprecated, it is clear that the documentation should be clear about:
  * Why it is required
  * When it should be used.

After some discussion in the ATK/AT-SPI Hackfest [1], we kind of agree that the "property-change" and the "notify" signals are definitely different in the sense they provide a different potential, even though we're not sure whether such a potential is currently being properly used since:

 - Sometimes implementors seem not to be filling the old_value in the AtkPropertyValues struct passed along with the "property-change" signal, even when they seem to be able to do it so.

 - Mike Gorse commented that the old values are being plainly ignored by at-spi2 because it just gets the new value from the struct and send it alone through D-Bus, so even though implementors had set the old_value, it would get lost at that point.

 - It looks like it makes sense to _always_ send something of type AtkPropertyValues as the arg1 parameter for the 'property-change' signal, so we wonder why the callback defines a gpointer instead of forcing an AtkPropertyValues type. Perhaps there are some cases where it could make sense to send something else.

So, conclusions so far would basically be:

  - Investigate a little bit more about the origins and reasons behind this 'property-change' signal, and check whether the arg1 parameter could be changed from a gpointer type to a AtkPropertyValues type.

  - Check if implementors that could be filling the old value in that struct are or ignoring it, and if that's the case, fill bugs for those.

  - It would be great to fix at-spi2 so it doesn't keep ignoring the old value when re-emitting the signal through D-Bus

[1] https://live.gnome.org/Hackfests/ATK2011

Additional info:

GObject::notify is being redefined, and it uses atk_object_notify.

In summary it emits the signal "property-change", filling the new_value field and not taking care of the old value.

So in the end it seems that this signals was added in order to extend the notify signal, and emit the property change with an old and a new value.

Anyway, Mario also pointed that it would be difficult to emit this "property-change" with the new and old value without triggering the atk_object_notify, that would implement this.

Now I think that it wIt is clear that we need to sort out how this signal is supposed to be used, and improve the documentation of that signal.

[Mass-reassigning open atk bug reports for better trackability as requested in https://bugzilla.gnome.org/show_bug.cgi?id=653179 .
PLEASE NOTE:
If you have watched the previous assignee of this bug report as a workaround for actually getting notified of changes in atk bugs, you yourself will now have to add atk-maint@gnome.bugs to your watchlist at the bottom of https://bugzilla.gnome.org/userprefs.cgi?tab=email to keep watching atk bug reports in GNOME Bugzilla.
Sorry for the noise: Feel free to filter for this comment in order to mass-delete the triggered bugmail.]

More about this: bug 666210

I have been reviewing this during webkitgtk2013 hackfest.

(In reply to comment #0)

> Whose purpose overlaps. After think a little if having property-change has any
> advantage, I only can think on:
>    * Having property-change allow to have a better control about when emit the
> signal, as "notify" docs explicitly say that a emission of that signal doesn't
> means always a change on the property.
>    * Right now property-change is only used on a subset of all the AtkObject
> properties. So having this signal adds a extra flexibility here. But not sure
> if this is a advantage, as you can connect to specific details on the "notify"
> signal.

As I say on comment 4, property-change is basically a GObject:notify forward. But I found the reason of this forward. GObject:notify doesn't allow hooks (is defined as NO_HOOK [1]). That means that you can't listen to this signal using atk_add_global_event_listener, that is the procedure used to forward the event via DBUS.

> So, should we deprecate AtkObject "property-change" and use GObject "notify"
> instead?

We can't deprecate "property-change", as right now we depend on global event listening.


> (In reply to comment #1)

> If finally this signal is not deprecated, it is clear that the documentation
> should be clear about:
>   * Why it is required

Now we know why.

>   * When it should be used.

Implementors shouldn't use it. It is just an AtkObject (and subclasses) notify event forwarding procedure. That needs to be documented.


(In reply to comment #3)
> After some discussion in the ATK/AT-SPI Hackfest [1], we kind of agree that the
> "property-change" and the "notify" signals are definitely different in the
> sense they provide a different potential, even though we're not sure whether
> such a potential is currently being properly used since:
> 
>  - Sometimes implementors seem not to be filling the old_value in the
> AtkPropertyValues struct passed along with the "property-change" signal, even
> when they seem to be able to do it so.
> 
>  - Mike Gorse commented that the old values are being plainly ignored by
> at-spi2 because it just gets the new value from the struct and send it alone
> through D-Bus, so even though implementors had set the old_value, it would get
> lost at that point.

old value seems to be just not used at all: lets document that.

>  - It looks like it makes sense to _always_ send something of type
> AtkPropertyValues as the arg1 parameter for the 'property-change' signal, so we
> wonder why the callback defines a gpointer instead of forcing an
> AtkPropertyValues type. Perhaps there are some cases where it could make sense
> to send something else.

Using something that is not an AtkPropertyValues is a bad idea. at-spi2-atk assumes an AtkPropertyValues. We will document that an AtkPropertyValues is supposed.

> So, conclusions so far would basically be:

This signal is needed. Need to improve documentation.

[1] https://developer.gnome.org/gobject/unstable/gobject-Signals.html#GSignalFlags

Created attachment 263929 [details] [review]
Improve documentation for AtkObject::property-change

This patch improves the documentation, and clarifies why it is needed, how it should be used, and who should be used. I think that it is enough to close the bug.

Review of the patch, specifically the wording is welcome.

Review of attachment 263929 [details] [review]:

I think the documentation makes now clear why that signal is there and why it should be normally ignored, so looks good enough to me :)

Comment on attachment 263929 [details] [review]
Improve documentation for AtkObject::property-change

Reviewed by Joanmarie Diggs on IRC

Two reviews. Committed after some changes proposed by Joanmarie Diggs. Closing bug.