GNOME Bugzilla – Bug 704621
Accessible link text for links in multi-page documents is incorrect
Last modified: 2013-07-30 17:27:04 UTC
Created attachment 249709 [details] event listener Created an attachment (id=249719) test case Steps to reproduce: 1. Open a PDF with links on the first page. 2. Enable caret navigation (see bug 702079) 3. Run the attached listener in a terminal. 4. Arrow from the first page to the second page. Expected results: When arrowing to the second page, the text of that link would be printed. Actual results: When arrowing to the second page, a substring from the first page is printed.
Created attachment 250479 [details] [review] Clear the ev-view-accessible cache when updating the internal current page When asking for the getNLinks() the current_page in the EvViewAccessible private data was updated but the cached textbuffer was not updated, so the text returned was the one that corresponds to the previous current page
Created attachment 250484 [details] [review] Clear the ev-view-accessible cache when updating the current page or document Improved patch. Clear the cache when the current page or document changes, there is no need to maintain the current_page in the private structure of EvViewAccessible.
Review of attachment 250484 [details] [review]: Looks good, but I have some questions ::: libview/ev-view-accessible.c @@ +1098,3 @@ + +static void +page_changed_cb (EvDocumentModel *model G_GNUC_UNUSED, Do we need this G_GNUC_UNUSED? does it fail to build or produce a warning when not used? @@ +1103,3 @@ + EvViewAccessible *accessible) +{ + if (old_page != new_page) This signal is never emitted with old_page == new_page. This is used to know from which page we have jumped without having to keep track of previous page. So you can simply forget old page here. @@ +1134,3 @@ + view = EV_VIEW (widget); + if (view->model) { What happens if the model hasn't been set at this point? Even though in practice the model of the view never changes (at least not in evince) ev_view_set_model can be called multiple times for different models. Maybe we could add ev_view_accessible_set_model and call it from ev_view_set_model.
Created attachment 250488 [details] [review] Clear the ev-view-accessible cache when updating the current page or document Patch updated according to comments.
Comment on attachment 250488 [details] [review] Clear the ev-view-accessible cache when updating the current page or document Thanks, pushed with some minor details changed