Created attachment 256204 [details] [review]
First version of the signal

First version of the signal. This is just a page-changed signal, without any parameters. Anyway I can think on some parameters:
  * prev_page_number: page number just before the signal emission
  * new_page_number: page number just after the signal emission

Anyway, imho, if we add parameters I think that just the new page number would be enough.

Note: to the native english speakers reading this:
1.) I based the signal documentation on the bug description feedback.
2.) Documenting the parameters, I documented @atkdocument as "the object which emitted the signal". But on the other AtkDocument signals, it is "the object which received the signal". For me it makes more sense the one I used. And after all, the implementors will need to call g_signal_emit with that instance. Looking at gtk, they have the following:
   a.) "@accel_group: the #GtkAccelGroup which received the signal" (gtkaccelgroup)
   b.) "label: The object on which the signal was emitted" (gtkaboutdialog)
   c.) "@application: the #GtkApplication which emitted the signal" (gtkapplication)

So they are not consistent. Taking into account g_signal_emit documentation:
"instance : the instance the signal is being emitted on."

Probably b.) is the more suitable one.

(In reply to comment #1)
> Created an attachment (id=256204) [details] [review]

Nit on the docs:

+       * The 'page-changed' signal is emitted when the current page of
+       * a document changes, e.g. pressing page up/down on a document
+       * viewer.

s/on a document/in a document/
 
> First version of the signal. This is just a page-changed signal, without any
> parameters. Anyway I can think on some parameters:
>   * prev_page_number: page number just before the signal emission
>   * new_page_number: page number just after the signal emission

Knowing what the current page number is would be quite helpful. The use case is that Orca could present the new page number to the user.

Knowing what the old page number is might not be needed/used. I cannot think of a use case right now. And requiring both might be more work for the implementor depending on the implementation.

So... I'm leaning towards just one parameter: The number of the newly-selected page. And if for some reason the implementor doesn't know that (or numbers don't apply or whatever), -1 should be provided.

> Anyway, imho, if we add parameters I think that just the new page number would
> be enough.

Agreed. :)

> 2.) Documenting the parameters, I documented @atkdocument as "the object which
> emitted the signal".

Makes sense to me.

> Probably b.) is the more suitable one.

Agreed. :)

Thanks!!

Created attachment 256486 [details] [review]
Add page-changed signal

Adds signal AtkDocument::page-changed. It adds one parameter page-number

Created attachment 256488 [details] [review]
tests: add testdocument

In order to try the signal (basically that I didn't commit any mistake with it) I created a little program. I modified it in order to be included on testdocuments. But right now is somewhat silly, as in order to check if the test fails or not, it only check that the expected number of callback calls is the same that the expected number of signal emission.

Anyway, right now the tests that atk have at the tests directory are also really shallow, so I'm biased to just add it, and improve it later. Anyway feedback are welcome.

(In reply to comment #3)
> Created an attachment (id=256486) [details] [review]
> Add page-changed signal
> 
> Adds signal AtkDocument::page-changed. It adds one parameter page-number

BTW: I updated the documentation, and also added page_number documentation. Native-speaking feedback are welcome.