[zathura] [Zathura PATCH 2/2] Hide the inputbar and completion menu before saving the adjustments ratios into a new jump structure

[Marwan Tanager]

Since we are now saving the adjustments ratios in the jump structures, we need
to take care of the following scenario:

- We do an action that results in a new jump structure being added to the
jumplist while the inputbar is visible (e.g., search, jumping to a
specific page, jumping to a bookmark, or following a link).

- Since we are now storing the adjustments ratios in the jump structures,
all of the above actions would result in the vertical adjustment ratio
being saved while the inputbar and/or the completion menu is visible.

- Now we are exactly on the target of the jump (note that the inputbar and
completion menu now are hidden), so suppose that we want to go back using
^o (assuming that we didn't change the adjustments after jumping), then
the check at sc_jumplist that compares the current adjustments ratios
with that of the current jump (the jump that has just been added and
which we are currently on it's position) would fail, because after the
inputbar (with possibly the completion menu in case of bookmarks) is
activated it is hidden, which results in the vertical adjustment upper
bound to change, which in turn results in the vertical adjustment ratio
returned by zathura_adjustment_get_ratio to become different from what is
stored in the current jump structure, even though we haven't changed the
adjustments at all after the jump. This would always result in taking us
back to the exact position of the jump (which would be slightly different
from the current position) when we press ^o. This can be annoying,
because it would happen, for example, every time we need to go back
quickly after jumping to a link target, a search result, or a bookmark.

So, what this patch does is essentially to make the vertical adjustment ratio
reflecting the current vertical adjustment after a jump, to always be the same
as the one stored in the newly added jump structure, since both are calculated
with zathura_adjustment_get_ratio while the inputbar is _not_ visible, so they
should be the same.

I've elaborated just to make things clear, in case the purpose of the patch
isn't obvious.

---
callbacks.c | 6 +-----
commands.c | 1 +
links.c | 5 +++++
shortcuts.c | 1 +
zathura.c | 17 +++++++++++++++++
zathura.h | 7 +++++++
6 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/callbacks.c b/callbacks.c
index 1212b1d..191662b 100644
--- a/callbacks.c
+++ b/callbacks.c
@@ -274,10 +274,7 @@ cb_index_row_activated(GtkTreeView* tree_view, GtkTreePath* path,
}

sc_toggle_index(zathura->ui.session, NULL, NULL, 0);
-
- /* zathura_jumplist_save is called when entering index mode */
zathura_link_evaluate(zathura, index_element->link);
- zathura_jumplist_add(zathura);
}

g_object_unref(model);
@@ -333,9 +330,7 @@ handle_link(GtkEntry* entry, girara_session_t* session,
invalid_index = false;
switch (action) {
case ZATHURA_LINK_ACTION_FOLLOW:
- zathura_jumplist_add(zathura);
zathura_link_evaluate(zathura, link);
- zathura_jumplist_add(zathura);
break;
case ZATHURA_LINK_ACTION_DISPLAY:
zathura_link_display(zathura, link);
@@ -534,6 +529,7 @@ cb_unknown_command(girara_session_t* session, const char* input)
}
}

+ zathura_hide_inputbar(zathura);
zathura_jumplist_add(zathura);
page_set(zathura, atoi(input) - 1);
zathura_jumplist_add(zathura);
diff --git a/commands.c b/commands.c
index 76e2a27..2bd6082 100644
--- a/commands.c
+++ b/commands.c
@@ -117,6 +117,7 @@ cmd_bookmark_open(girara_session_t* session, girara_list_t* argument_list)
return false;
}

+ zathura_hide_inputbar(zathura);
zathura_jumplist_add(zathura);
if (bookmark->x != DBL_MIN && bookmark->y != DBL_MIN) {
GtkAdjustment* hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(zathura->ui.session->gtk.view));
diff --git a/links.c b/links.c
index 968b983..e938aee 100644
--- a/links.c
+++ b/links.c
@@ -149,6 +149,9 @@ zathura_link_evaluate(zathura_t* zathura, zathura_link_t* link)
}
}

+ zathura_hide_inputbar(zathura);
+ zathura_jumplist_add(zathura);
+
/* jump to the page */
page_set(zathura, link->target.page_number);

@@ -161,6 +164,8 @@ zathura_link_evaluate(zathura_t* zathura, zathura_link_t* link)
} else {
position_set(zathura, -1, offset.y);
}
+
+ zathura_jumplist_add(zathura);
}
break;
case ZATHURA_LINK_GOTO_REMOTE:
diff --git a/shortcuts.c b/shortcuts.c
index 388dfc3..001eb70 100644
--- a/shortcuts.c
+++ b/shortcuts.c
@@ -1005,6 +1005,7 @@ sc_search(girara_session_t* session, girara_argument_t* argument,
zathura_rectangle_t rectangle = recalc_rectangle(target_page, *rect);
page_offset_t offset;
page_calculate_offset(zathura, target_page, &offset);
+ zathura_hide_inputbar(zathura);
zathura_jumplist_add(zathura);

if (zathura_page_get_index(target_page) != cur_page) {
diff --git a/zathura.c b/zathura.c
index a82fb55..af492b2 100644
--- a/zathura.c
+++ b/zathura.c
@@ -13,6 +13,7 @@
#include <girara/session.h>
#include <girara/statusbar.h>
#include <girara/settings.h>
+#include <girara/shortcuts.h>
#include <glib/gstdio.h>
#include <glib/gi18n.h>

@@ -1145,6 +1146,22 @@ position_set(zathura_t* zathura, double position_x, double position_y)
}
}

+void zathura_hide_inputbar(zathura_t* zathura)
+{
+ g_return_if_fail(zathura != NULL && zathura->ui.session->gtk.inputbar != NULL);
+
+ girara_argument_t arg = { GIRARA_HIDE, NULL };
+ girara_isc_completion(zathura->ui.session, &arg, NULL, 0);
+
+ if (zathura->ui.session->global.autohide_inputbar == true) {
+ gtk_widget_hide(zathura->ui.session->gtk.inputbar);
+ }
+
+ while (gtk_events_pending()) {
+ gtk_main_iteration();
+ }
+}
+
bool
zathura_jumplist_has_previous(zathura_t* zathura)
{
diff --git a/zathura.h b/zathura.h
index c67446d..7f0adfb 100644
--- a/zathura.h
+++ b/zathura.h
@@ -335,6 +335,13 @@ void position_set(zathura_t* zathura, double position_x, double position_y);
void page_widget_set_mode(zathura_t* zathura, unsigned int pages_per_row, unsigned int first_page_column);

/**
+ * Hide the inputbar and the completion menu immediately
+ *
+ * @param zathura The zathura session
+ */
+void zathura_hide_inputbar(zathura_t* zathura);
+
+/**
* Updates the page number in the statusbar. Note that 1 will be added to the
* displayed number
*

[Sebastian Ramacher]

Post by Marwan Tanager
+void zathura_hide_inputbar(zathura_t* zathura)
+{
+ g_return_if_fail(zathura != NULL && zathura->ui.session->gtk.inputbar != NULL);
+
+ girara_argument_t arg = { GIRARA_HIDE, NULL };
+ girara_isc_completion(zathura->ui.session, &arg, NULL, 0);
+
+ if (zathura->ui.session->global.autohide_inputbar == true) {
+ gtk_widget_hide(zathura->ui.session->gtk.inputbar);
+ }
+
+ while (gtk_events_pending()) {
+ gtk_main_iteration();
+ }

This always looks like a hack to me. I'll play around with the patches a
bit. I'd prefer it to be solved differently but if it can't be done,
we'll probably need to go down this road.

Would it be possible to calculate the aspect ratio differently, i.e. be
honoring the inputbar if it is visible?

Regards

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager
+void zathura_hide_inputbar(zathura_t* zathura)
+{
+ g_return_if_fail(zathura != NULL && zathura->ui.session->gtk.inputbar != NULL);
+
+ girara_argument_t arg = { GIRARA_HIDE, NULL };
+ girara_isc_completion(zathura->ui.session, &arg, NULL, 0);
+
+ if (zathura->ui.session->global.autohide_inputbar == true) {
+ gtk_widget_hide(zathura->ui.session->gtk.inputbar);
+ }
+
+ while (gtk_events_pending()) {
+ gtk_main_iteration();
+ }

This always looks like a hack to me. I'll play around with the patches a
bit. I'd prefer it to be solved differently but if it can't be done,
we'll probably need to go down this road.

I agree with you that it's a bit hacky, but I don't think that it does any
harm; it is just handling of a special case, so I don't think that it needs to
be more abstract.

Post by Sebastian Ramacher
Would it be possible to calculate the aspect ratio differently, i.e. be
honoring the inputbar if it is visible?

I think it would, but only if we know how to map (i.e., scale) the vertical
allocation of the completion menu and/or the inputbar, which would be typically
in pixels, to a floting-point adjustment value, so that we could add it to the
upper bound before calculating the ratio, but currently, I don't know how could
this be done.

Anyway, I've simplifed the patch further in version 2 (and moved some cleanups
that I've made along the way to a separate patch), by hiding the
zathura_hide_inputbar (now, zathura_jumplist_hide_inputbar) function entirely,
and just calling it from zathura_jumplist_save, so that instead of changing 6
files, only 1 is changed now. This makes this patch more attached to just this
special case, so that it is more unlikely that it would get abused in some
unanticipated ways, plus eliminating all the zathura_hide_inputbar calls that
could probably hinder the logical flow of the code.

Feel free to NAK it, if you disagree.


--
Marwan