Created attachment 178494 [details] [review]
First atkhypertext implementation

This patch implement AtkHypertext in ev-view-accessible object. To implement that I've created two new objects, ev-view-accessible-hyperlink and ev-view-accessible-hyperlink-impl.

I see this patch is based in the first version of a similar one for WebKitGTK [1], proposed to fix bug 33785 [2]. The problem with that patch is that is obsolete and wrong, so that's why it didn't get accepted :-)... however there's a newer one which got integrated some time ago in wkgtk, so I guess you could be interested in taking a look to it to update this patch taking some ideas from that last version (which, among other things, doesn't use that ugly hash table):

https://bugs.webkit.org/attachment.cgi?id=72159&action=prettypatch

Hope this helps,
Mario

[1] https://bugs.webkit.org/attachment.cgi?id=71301&action=prettypatch
[2] https://bugs.webkit.org/show_bug.cgi?id=33785

Created attachment 186338 [details] [review]
AtkHypertext implementation

Fixing some details that was based on an old AtkHypertext patch to webkitgtk

It looks to me like you're attaching three different patches through a single file in bugzilla, which is not a very convenient way IMHO to ask for review. 

Instead, if you want to keep those changes in different patches you should attach them separately, so every different change was easier to review.

Also I noticed that you have changed (renamed and/or removed), in PATCH 3/3, some code that you introduced as new in PATCH 2/3, which also adds some unnecessary complexity to the review process of this patch. For instance, you introduce a method ev_view_accessible_hyperlink_get_or_create() in patch 2/3 (using a hash table for internal purposes) that you just replace with another method, named  ev_view_accessible_hyperlink_new() (which doesn't use that hash table), in patch 3/3.

Thus, I think you certainly should at least squash those two last patches into a single one before asking for review over them.

Last, some extra nitpicking sorry :-)... I saw you credited the author (that is, me! :-)) of the original patch through lines like these in patch 3/3:

  diff --git a/libview/ev-view-accessible-hyperlink.h b/libview/ev-view-  accessible-hyperlink.h
  index 9ac45ac..dfadb74 100644
  --- a/libview/ev-view-accessible-hyperlink.h
  +++ b/libview/ev-view-accessible-hyperlink.h
  @@ -19,6 +19,11 @@
  * Boston, MA 02110-1301, USA.
  */
 
  +/*
  + * Based on WebKitGtk AtkHyperlink implementation by msanchez
  + * (msanchez@igalia.com)
  + *

I'd say that "by msanchez" is not very descriptive :-). "If I were the author", I think I'd strongly prefer my name instead. Something like "Mario Sanchez Prada" would probably work fine, I guess :-)

Wrapping up, if you could provide separate atachments for every patch and squash the last two ones together, that would be great for reviewing them.

Thanks!

Created attachment 186416 [details] [review]
atkhypertext interface

Created attachment 186417 [details] [review]
Implementation of atkhyperlink and atkhypertext

I've just splitted the patch in two, as Mario suggests for an easier review. The first one is the implementation of atkhypertext in ev-view-accessible object that does nothing, only function definition. And the second one is the real implementation, based on webkit implementation, creating two new atkobjects, ev-view-accessible-hyperlink and ev-iew-accessible-hyperlink-impl, which are used to implement atkhypertext interface.

Ping. :-) Is this reviewable now? (Please and thank you in advance!)

Sorry for forgetting about this for so long. Reviewed now and it looks good to me, although someone else from Evince (Carlos?) should approve the commit I guess.

Time for my annual ping. :-)

Started from scratch and pushed a couple of patches to git master, please, let me know if there's any issue