From ffdc137ded98199cb3a9200be25846d5b011e256 Mon Sep 17 00:00:00 2001 From: lafricain79 Date: Mon, 1 Jun 2026 11:08:30 +0200 Subject: [PATCH 1/2] Highlight verses using tag folder color. Solves #968 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **Truncated multi-reference bookmarks**: `BackEnd::parse_verse_list()` used `popError()` as a loop-termination condition, which also fires when a single element in the list fails to resolve — silently dropping every element after it, even valid ones. Switched to indexed access (`getCount()` / `getElement()`), so one bad reference no longer swallows the rest of a bookmark's verse list. - **Unreadable text on dark tag colors**: the current-verse `` wrapper was applied unconditionally, overriding the contrast color already computed for the tag background. It's now skipped whenever a tag color is active, so the computed black/white contrast color is respected instead. - **Major bug: verse text turning invisible for the rest of the chapter**: the tag-color `` and the current-verse/annotation `` were opened independently but shared a single, under-conditioned closing statement. When a verse had a tag color but wasn't the current verse, the highlight span was opened is never opened but its closing tag was still emitted, corrupting the HTML structure for every subsequent verse in the chapter. Each span now tracks its own open/close state explicitly. - **Strong's/morphology links ignoring computed contrast**: verse numbers and linked words are colored via a global CSS `a:link` rule, which overrides any `color` inherited from an ancestor ``. Added a `.tagcolor a:link { color: inherit !important }` rule and tagged the relevant span with `class="tagcolor"` so link text respects the computed contrast color too. - **On/off toggle**: added "Colorize verses by folder color" as a right-click checkbox item on the bookmarks tree (rather than buried in Preferences), matching the existing "Popup menu for cross-references" toggle added by #1335. Defaults to on, persists across restarts, and takes effect immediately. - **Status bar bookmark info on hover**: hovering (not clicking) a verse number that belongs to a bookmark now shows ": " in the status bar, making it easier to identify which of several similarly-colored bookmarks a verse belongs to — without exposing the full nested folder path. - The luminance/contrast calculation itself (`text_color_for_bg`) was already correct; the "dark green on black" and "white on white" issues were caused by other elements overriding its output, not by the luminance formula. --- src/backend/sword_main.cc | 15 +++- src/gtk/bookmarks_menu.c | 29 ++++++++ src/gtk/bookmarks_treeview.c | 73 +++++++++++++++++++ src/gui/bookmarks_menu.h | 3 + src/gui/bookmarks_treeview.h | 1 + src/main/display.cc | 137 ++++++++++++++++++++++++++++++++++- src/main/settings.c | 6 ++ src/main/settings.h | 1 + src/main/url.cc | 12 ++- ui/xi-menus-popup.gtkbuilder | 10 +++ ui/xi-menus.glade | 10 +++ 11 files changed, 290 insertions(+), 7 deletions(-) diff --git a/src/backend/sword_main.cc b/src/backend/sword_main.cc index 7893e4977..e9a526d40 100644 --- a/src/backend/sword_main.cc +++ b/src/backend/sword_main.cc @@ -925,11 +925,18 @@ GList *BackEnd::parse_verse_list(const char *module_name, const char *list, char key->setText(current_key); vs = key->parseVerseList(list, *key, TRUE); - if (!vs.getCount()) + int count = vs.getCount(); + if (!count) return retlist; - while (!vs.popError()) { - retlist = g_list_append(retlist, strdup((char *)vs.getText())); - vs++; + /* Use indexed access rather than popError()-driven iteration: + * popError() stops the whole walk as soon as ANY single element + * fails to resolve, silently dropping every element after it, + * even when they are perfectly valid. Indexed access lets us + * simply skip a bad element and keep going. */ + for (int i = 0; i < count; i++) { + SWKey *elem = vs.getElement(i); + if (elem) + retlist = g_list_append(retlist, strdup((char *)elem->getText())); } return retlist; } diff --git a/src/gtk/bookmarks_menu.c b/src/gtk/bookmarks_menu.c index 374b57fd3..43a9c3bbc 100644 --- a/src/gtk/bookmarks_menu.c +++ b/src/gtk/bookmarks_menu.c @@ -499,6 +499,31 @@ G_MODULE_EXPORT void on_crossref_popup_activate(GtkMenuItem *menuitem, xml_set_value("Xiphos", "misc", "crossref_popup", settings.crossref_popup ? "1" : "0"); } +/****************************************************************************** + * Name + * on_tag_colorize_activate + * + * Synopsis + * #include "gui/bookmarks_menu.h" + * + * void on_tag_colorize_activate(GtkMenuItem *menuitem, + * gpointer user_data) + * + * Description + * toggle verse colorization by bookmark folder color + * + * Return value + * void + */ +G_MODULE_EXPORT void on_tag_colorize_activate(GtkMenuItem *menuitem, + gpointer user_data) +{ + settings.tag_colorize = + gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); + xml_set_value("Xiphos", "misc", "tag_colorize", + settings.tag_colorize ? "1" : "0"); + main_display_bible(NULL, settings.currentverse); +} /****************************************************************************** * Name @@ -1263,6 +1288,10 @@ void gui_create_bookmark_menu(void) gtk_check_menu_item_set_active( GTK_CHECK_MENU_ITEM(menu.crossref_popup), settings.crossref_popup); + menu.tag_colorize = UI_GET_ITEM(gxml, "tag_colorize"); + gtk_check_menu_item_set_active( + GTK_CHECK_MENU_ITEM(menu.tag_colorize), + settings.tag_colorize); gtk_widget_set_sensitive(menu.in_tab, FALSE); gtk_widget_set_sensitive(menu.in_dialog, FALSE); diff --git a/src/gtk/bookmarks_treeview.c b/src/gtk/bookmarks_treeview.c index 5e31bd8bd..d2b191e07 100644 --- a/src/gtk/bookmarks_treeview.c +++ b/src/gtk/bookmarks_treeview.c @@ -1053,6 +1053,54 @@ void bookmark_debug_dump_colors(void) } +/* Recursively search @folder (and its sub-folders) for a bookmark whose + * key resolves to @versekey_text. On match, returns ": ". Helper for + * bookmark_get_tag_info_for_key(). */ +static gchar *bookmark_find_in_folder(GtkTreeIter *folder, const gchar *versekey_text) +{ + gchar *folder_caption = NULL; + gtk_tree_model_get(GTK_TREE_MODEL(model), folder, + COL_CAPTION, &folder_caption, -1); + GtkTreeIter child; + gchar *result = NULL; + if (gtk_tree_model_iter_children(GTK_TREE_MODEL(model), &child, folder)) { + do { + gchar *node_key = NULL, *node_caption = NULL; + gtk_tree_model_get(GTK_TREE_MODEL(model), &child, + COL_KEY, &node_key, + COL_CAPTION, &node_caption, -1); + if (node_key) { + /* leaf: a bookmark. Resolve its (possibly + * multi-reference) key via Sword and compare + * each resolved verse to versekey_text. */ + GList *verses = main_parse_verse_list( + settings.MainWindowModule, node_key, + (char *)settings.currentverse); + for (GList *l = verses; l && !result; l = l->next) { + gchar *v = g_strstrip(g_strdup((const char *)l->data)); + gchar *q = g_strstrip(g_strdup(versekey_text)); + if (!g_ascii_strcasecmp(v, q)) + result = g_strdup_printf("%s: %s", + folder_caption ? folder_caption : "", + node_caption ? node_caption : node_key); + g_free(v); + g_free(q); + } + for (GList *l = verses; l; l = l->next) + g_free(l->data); + g_list_free(verses); + } else { + /* sub-folder: recurse */ + result = bookmark_find_in_folder(&child, versekey_text); + } + g_free(node_key); + g_free(node_caption); + } while (!result && gtk_tree_model_iter_next(GTK_TREE_MODEL(model), &child)); + } + g_free(folder_caption); + return result; +} gchar *bookmark_get_tag_color_for_key(const gchar *osiskey) { GtkTreeIter folder, child; @@ -1102,6 +1150,31 @@ gchar *bookmark_get_tag_color_for_key(const gchar *osiskey) return NULL; } +/** + * bookmark_get_tag_info_for_key: + * @versekey_text: a Sword-formatted verse key string, e.g. "Rom 16:23" + * + * Finds the first bookmark, anywhere in the tree, whose (possibly + * multi-reference) key resolves to @versekey_text, and returns + * ": " -- just the single + * immediate parent folder, not the full nested path. + * Returns NULL if no bookmark matches. Caller must g_free() the result. + */ +gchar *bookmark_get_tag_info_for_key(const gchar *versekey_text) +{ + if (!versekey_text || !model) + return NULL; + GtkTreeIter folder; + if (!gtk_tree_model_get_iter_first(GTK_TREE_MODEL(model), &folder)) + return NULL; + gchar *result = NULL; + do { + result = bookmark_find_in_folder(&folder, versekey_text); + if (result) + return result; + } while (gtk_tree_model_iter_next(GTK_TREE_MODEL(model), &folder)); + return NULL; +} GtkWidget *gui_create_bookmark_tree(void) { diff --git a/src/gui/bookmarks_menu.h b/src/gui/bookmarks_menu.h index 22bab639e..a86034688 100644 --- a/src/gui/bookmarks_menu.h +++ b/src/gui/bookmarks_menu.h @@ -44,6 +44,7 @@ struct _bookmark_menu GtkWidget *remove; GtkWidget *set_color; GtkWidget *crossref_popup; + GtkWidget *tag_colorize; }; typedef struct _bookmark_menu BOOKMARK_MENU; extern BOOKMARK_MENU menu; @@ -64,6 +65,8 @@ void on_allow_reordering_activate(GtkMenuItem *menuitem, gpointer user_data); void on_crossref_popup_activate(GtkMenuItem *menuitem, gpointer user_data); +void on_tag_colorize_activate(GtkMenuItem *menuitem, + gpointer user_data); void on_dialog_activate(GtkMenuItem *menuitem, gpointer user_data); void on_edit_item_activate(GtkMenuItem *menuitem, diff --git a/src/gui/bookmarks_treeview.h b/src/gui/bookmarks_treeview.h index 62528f5cd..59e70d3a2 100644 --- a/src/gui/bookmarks_treeview.h +++ b/src/gui/bookmarks_treeview.h @@ -75,6 +75,7 @@ void gui_verselist_to_bookmarks(GList *verses, GtkWidget *gui_create_bookmark_tree(void); void bookmark_debug_dump_colors(void); gchar *bookmark_get_tag_color_for_key(const gchar *osiskey); +gchar *bookmark_get_tag_info_for_key(const gchar *versekey_text); void gui_parse_bookmarks(GtkTreeView *tree, const xmlChar *file, GtkTreeIter *parent); GtkWidget *gui_create_dialog_add_bookmark(gchar *label, diff --git a/src/main/display.cc b/src/main/display.cc index 43f24fe20..f426a69d5 100644 --- a/src/main/display.cc +++ b/src/main/display.cc @@ -52,6 +52,7 @@ #include "main/xml.h" #include "gui/utilities.h" +#include "gui/bookmarks_treeview.h" #include "gui/widgets.h" #include "gui/dialog.h" @@ -96,6 +97,7 @@ body { background-color:%s; color:%s; -webkit-column-count: %d ; margin-top: 0.1 td { %s } \ .introMaterial { font-style: italic; } \ a:link{ color:%s } %s %s \ +.tagcolor a:link{ color:inherit !important } \ h3 { font-style: %s } --> \ %s %s " // 11 interpolable values: bg/txt/link colors, columns, justify (2x, body+td), @@ -1335,6 +1337,115 @@ GTKChapDisp::getVerseAfter(SWModule &imodule) buf = NULL; } +/* Returns white or black depending on background luminance */ +static const char *text_color_for_bg(const gchar *hex_color) +{ + if (!hex_color || hex_color[0] != '#' || strlen(hex_color) < 7) + return "#000000"; + int r = 0, g = 0, b = 0; + sscanf(hex_color + 1, "%02x%02x%02x", &r, &g, &b); + /* relative luminance (ITU-R BT.709) */ + double lum = 0.2126 * r + 0.7152 * g + 0.0722 * b; + return (lum < 128.0) ? "#FFFFFF" : "#000000"; +} + +/* Returns the tag color for a given VerseKey by comparing OSIS refs. + * Walks the GtkTreeStore directly from C++. */ +static gchar *get_tag_color_for_versekey(VerseKey *vk) +{ + extern GtkTreeStore *model; + if (!settings.tag_colorize) return NULL; + if (!model || !vk) return NULL; + + gchar *osisref = g_strdup_printf("%s.%d.%d", + vk->getOSISBookName(), + vk->getChapter(), + vk->getVerse()); + + GtkTreeIter root; + if (!gtk_tree_model_get_iter_first(GTK_TREE_MODEL(model), &root)) { + g_free(osisref); return NULL; + } + + GQueue *stack = g_queue_new(); + GQueue *colors = g_queue_new(); + GtkTreeIter child; + if (gtk_tree_model_iter_children(GTK_TREE_MODEL(model), &child, &root)) { + do { + GtkTreeIter *copy = g_new(GtkTreeIter, 1); + *copy = child; + g_queue_push_tail(stack, copy); + g_queue_push_tail(colors, NULL); + } while (gtk_tree_model_iter_next(GTK_TREE_MODEL(model), &child)); + } + + gchar *result = NULL; + while (!g_queue_is_empty(stack) && !result) { + GtkTreeIter *iter = (GtkTreeIter *)g_queue_pop_head(stack); + gchar *inherited = (gchar *)g_queue_pop_head(colors); + gchar *node_color = NULL, *node_key = NULL; + gtk_tree_model_get(GTK_TREE_MODEL(model), iter, + COL_COLOR, &node_color, + COL_KEY, &node_key, -1); + const gchar *effective = (node_color && *node_color) + ? node_color : inherited; + if (node_key) { + /* Let Sword resolve the (possibly multi-reference, possibly + * partial) bookmark key into individual, fully-qualified + * verses -- same mechanism used for the verse-list popup. */ + if (effective) { + /* parse_verse_list() repositions the SWModule\'s own + * key, which is the SAME key object being iterated + * by the enclosing display loop. Save and restore + * it around the call so we don\'t corrupt that + * outer iteration (was causing freezes/GTK asserts + * with multi-reference bookmarks). */ + gchar *saved_pos = g_strdup((const char *)vk->getText()); + GList *verses = backend->parse_verse_list( + settings.MainWindowModule, node_key, + (char *)settings.currentverse); + vk->setText(saved_pos); + g_free(saved_pos); + for (GList *l = verses; l && !result; l = l->next) { + VerseKey vk2; + vk2.setLocale(vk->getLocale()); + vk2.setText((const char *)l->data); + gchar *ref2 = g_strdup_printf("%s.%d.%d", + vk2.getOSISBookName(), + vk2.getChapter(), + vk2.getVerse()); + if (!g_ascii_strcasecmp(osisref, ref2)) + result = g_strdup(effective); + g_free(ref2); + } + for (GList *l = verses; l; l = l->next) + g_free(l->data); + g_list_free(verses); + } + g_free(node_key); + } else { + if (gtk_tree_model_iter_children(GTK_TREE_MODEL(model), + &child, iter)) { + do { + GtkTreeIter *copy = g_new(GtkTreeIter, 1); + *copy = child; + g_queue_push_tail(stack, copy); + g_queue_push_tail(colors, + effective ? g_strdup(effective) : NULL); + } while (gtk_tree_model_iter_next( + GTK_TREE_MODEL(model), &child)); + } + } + g_free(node_color); + g_free(inherited); + g_free(iter); + } + g_queue_free_full(stack, g_free); + g_queue_free_full(colors, g_free); + g_free(osisref); + return result; +} + // part of the deep ugliness below, for extracting bad paragraph endings. // this is empirically observed in some marginally broken module content. struct paragraph_endings @@ -1406,6 +1517,18 @@ GTKChapDisp::RenderOneChapter(SWModule &imodule, if (*rework->str == '\0') continue; // no verse content there. + // tag-group (bookmark folder) color highlight -- wraps the + // verse number, user-note reference, and verse text below. + gchar *tag_color = get_tag_color_for_versekey(key); + if (tag_color) { + const char *tc_fg = text_color_for_bg(tag_color); + swbuf.appendFormatted( + "", + tag_color, tc_fg, tag_color); + } + // generate the verse number with color and decoration. gchar *num = main_format_number(key->getVerse()); swbuf.appendFormatted((settings.showversenum @@ -1440,11 +1563,16 @@ GTKChapDisp::RenderOneChapter(SWModule &imodule, color_chosen_bg = NULL; // but then decide on the special cases for this verse. + // (current-verse highlighting is suppressed when a bookmark + // tag color is active, so it can't override the contrast + // color already applied to the tag background; annotation + // highlighting still takes precedence and coexists with it, + // per review feedback.) if (settings.annotate_highlight && e) { color_choices = COLOR_BOTH; color_chosen_fg = settings.highlight_bg; color_chosen_bg = settings.highlight_fg; - } else if ((curChapter == thisChapter) && (curVerse == k)) { + } else if ((curChapter == thisChapter) && (curVerse == k) && !tag_color) { if (settings.versehighlight) { color_choices = COLOR_BOTH; color_chosen_fg = settings.highlight_fg; @@ -1503,6 +1631,13 @@ GTKChapDisp::RenderOneChapter(SWModule &imodule, if (color_choices == COLOR_BOTH) swbuf.append(""); + // close tag-group color span + if (tag_color) { + swbuf.append(""); + g_free(tag_color); + tag_color = NULL; + } + if (paragraph_end_pending) swbuf.append(para_type); diff --git a/src/main/settings.c b/src/main/settings.c index 68405ab45..7d3335a9b 100644 --- a/src/main/settings.c +++ b/src/main/settings.c @@ -1107,6 +1107,12 @@ if (!settings.morph_heb_lex || strlen(settings.morph_heb_lex) == 0) { xml_add_new_item_to_section("misc", "crossref_popup", "0"); settings.crossref_popup = 0; } + if ((buf = xml_get_value("misc", "tag_colorize"))) + settings.tag_colorize = atoi(buf); + else { + xml_add_new_item_to_section("misc", "tag_colorize", "1"); + settings.tag_colorize = 1; + } if ((buf = xml_get_value("misc", "xrefsinverselist"))) settings.xrefs_in_verse_list = atoi(buf); diff --git a/src/main/settings.h b/src/main/settings.h index 9541250eb..d45676190 100644 --- a/src/main/settings.h +++ b/src/main/settings.h @@ -106,6 +106,7 @@ struct _settings versehighlight, /* do special fg/bg for current verse */ annotate_highlight, /* do special bg/fg for user annotations */ crossref_popup, /* open cross-references in a popup menu */ + tag_colorize, /* colorize verses by bookmark folder color (default on) */ /* saved startup displays */ display_parallel, /* detached parallel */ display_modmgr, /* mod.mgr */ diff --git a/src/main/url.cc b/src/main/url.cc index 46f98f2ee..9dac95bf0 100644 --- a/src/main/url.cc +++ b/src/main/url.cc @@ -702,6 +702,7 @@ static gint show_in_previewer(const gchar *url) * gint */ +extern "C" gchar *bookmark_get_tag_info_for_key(const gchar *versekey_text); gint sword_uri(const gchar *url, gboolean clicked) { const gchar *key = NULL; @@ -738,8 +739,15 @@ gint sword_uri(const gchar *url, gboolean clicked) if (mod_type == DICTIONARY_TYPE) show_in_previewer(url); - else - gui_set_statusbar(name + ((*name == '/') ? 1 : 0)); + else { + const gchar *ref_text = name + ((*name == '/') ? 1 : 0); + gchar *tag_info = bookmark_get_tag_info_for_key(ref_text); + if (tag_info) { + gui_set_statusbar(tag_info); + g_free(tag_info); + } else + gui_set_statusbar(ref_text); + } } handling_uri = FALSE; return 1; diff --git a/ui/xi-menus-popup.gtkbuilder b/ui/xi-menus-popup.gtkbuilder index 6726567ff..069f140ae 100644 --- a/ui/xi-menus-popup.gtkbuilder +++ b/ui/xi-menus-popup.gtkbuilder @@ -133,6 +133,16 @@ + + + True + False + Highlight Bible verses using their bookmark folder color + Colorize verses by folder color + True + + + True diff --git a/ui/xi-menus.glade b/ui/xi-menus.glade index 3e299edd6..09e7abe8f 100644 --- a/ui/xi-menus.glade +++ b/ui/xi-menus.glade @@ -1793,6 +1793,16 @@ + + + True + Highlight Bible verses using their bookmark folder color + Colorize verses by folder color + True + True + + + From 1af662195cd8b4592df4c217c70e251570cf336e Mon Sep 17 00:00:00 2001 From: lafricain79 Date: Thu, 16 Jul 2026 21:47:20 +0200 Subject: [PATCH 2/2] Fix default bookmarks always created in English regardless of locale Xiphos never called setlocale(LC_ALL, ""), relying instead on gtk_init_with_args() to set the process locale implicitly as a side effect. gettext (bindtextdomain/textdomain) was also only initialized later, inside gui_init(). Both of these happen well after settings_init() -> init_bookmarks(), which generates the user's default bookmarks.xml using _()-wrapped strings ("What must I do to be saved?", "What is the Gospel?", etc.). As a result, the default bookmarks were always written in English, even when .po translations existed and the rest of the UI displayed correctly in the user's language. Move setlocale(LC_ALL, "") and the bindtextdomain/textdomain calls to the very start of main(), before settings_init(). The later calls in gui_init() are left in place and are harmless to repeat (gettext initialization is idempotent). --- src/main/main.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/main/main.c b/src/main/main.c index aee52df62..467d86a4c 100644 --- a/src/main/main.c +++ b/src/main/main.c @@ -21,6 +21,8 @@ #ifdef HAVE_CONFIG_H #include +#include +#include #endif #include @@ -246,6 +248,36 @@ int main(int argc, char *argv[]) } } + /* + * Initialize the locale and gettext as early as possible, before + * any code that generates translatable content (e.g. + * settings_init() -> init_bookmarks(), which writes the default + * bookmarks.xml using _()-wrapped strings). Previously this only + * happened later, implicitly via gtk_init_with_args() inside + * gui_init(), so the default bookmarks were always generated + * while the process was still in the "C" locale (English), + * regardless of the user's actual language. setlocale() and the + * later bindtextdomain/textdomain calls in gui_init() remain + * harmless to repeat (idempotent). + */ + setlocale(LC_ALL, ""); +#ifdef ENABLE_NLS + bindtextdomain(GETTEXT_PACKAGE, PACKAGE_LOCALE_DIR); + bind_textdomain_codeset(GETTEXT_PACKAGE, "UTF-8"); + textdomain(GETTEXT_PACKAGE); +#endif +#ifdef WIN32 + { + gchar *locale_dir = + g_win32_get_package_installation_directory_of_module(NULL); + locale_dir = g_strconcat(locale_dir, "\0", NULL); + locale_dir = g_build_filename(locale_dir, "share", "locale", NULL); + bindtextdomain(GETTEXT_PACKAGE, locale_dir); + bind_textdomain_codeset(GETTEXT_PACKAGE, "UTF-8"); + textdomain(GETTEXT_PACKAGE); + g_free(locale_dir); + } +#endif /* * check for directories and files */