a08ccd7dd4930bff5bcf0a32bd35ec22db71215f markd Sat Aug 22 06:14:41 2026 -0700 Fix the browser.theme drop down, which has never applied a theme, and retire browser.background and browser.bgcolor from the example configs. refs #38158 themeDropDown built the menu label from the hg.conf key and posted that label back, while setThemeFromCart looks a theme up by key, so no entry carrying a sort prefix or an underscore could ever resolve. Broken since af6898753ba (2023) introduced that key format and changed the menu side without the lookup side. themeDropDown now passes the key suffix as the option value and the prettified label as the display text, using cgiMakeDropListWithVals. setThemeFromCart compared the generated link against "<>" instead of the config value, so the "no theme file" marker never fired. It also let an empty link from a missing CSS file overwrite browser.style with nothing. Both now test the right string and leave browser.style alone when there is no theme to apply. browser.background has not been read by any CGI since hBackgroundImage() and its callers were removed in 2012 (8e6fcda9f58), and browser.bgcolor has never been read at all, yet ex.hg.conf presented browser.background as a working setting. Commented both out with a note, in ex.hg.conf and the three browserbox configs. ex.hg.conf also documented two conflicting browser.theme formats in two places. The comma form stopped working in 2023. Consolidated to a single block that describes what the code actually does. diff --git src/hg/lib/cart.c src/hg/lib/cart.c index e22a6f491ca..f004a7f0e4e 100644 --- src/hg/lib/cart.c +++ src/hg/lib/cart.c @@ -3145,57 +3145,63 @@ { if (inWeb) webEnd(); /* this does googleAnalytics for a lot of CGIs */ else if (didCartHtmlStart) cartFooter(); else return; popWarnHandler(); } void setThemeFromCart(struct cart *cart) /* If 'theme' variable is set in cart: overwrite background with the one from * defined for this theme Also set the "styleTheme", with additional styles * that can overwrite the main style settings */ { -// Get theme from cart and use it to get background file from config; -// format is browser.theme.<name>=<stylesheet>[,<background>] +// Get theme from cart and use it to get the stylesheet from config; +// format is browser.theme.<name>=<cssFileInStyleDir>, where <name> may carry a +// sort prefix, browser.theme.3.Sans_Serif=theme-modern.css. A value of "<>" +// means "no theme file", leaving whatever browser.style set. char *cartTheme = cartOptionalString(cart, "theme"); // XXXX which setting should take precedence? Currently browser.theme does. char *styleFile = cfgOption("browser.style"); if (styleFile != NULL) { char buf[512]; safef(buf, sizeof(buf), "<link rel='stylesheet' href='%s' type='text/css'>", styleFile); char *copy = cloneString(buf); htmlSetStyleTheme(copy); // for htmshell.c, used by hgTracks webSetStyle(copy); // for web.c, used by hgc } if (isNotEmpty(cartTheme)) { char *themeKey = catTwoStrings("browser.theme.", cartTheme); styleFile = cfgOption(themeKey); freeMem(themeKey); - if (isEmpty(styleFile)) + // "<>" means "default settings" = "no file". Test styleFile, not the link: + // webCssLink returns html or an empty string, never the "<>" marker itself. + if (isEmpty(styleFile) || sameString(styleFile, "<>")) return; char * link = webCssLink(styleFile, FALSE); // resource file link wrapped in html - if (link != NULL && !sameOk(link, "<>")) // "<>" means "default settings" = "no file" + // An empty link means the file was not found. Leave browser.style alone rather + // than overwriting it with nothing. + if (isNotEmpty(link)) { htmlSetStyleTheme(link); // for htmshell.c, used by hgTracks webSetStyle(link); // for web.c, used by hgc } } } void cartSetLastPosition(struct cart *cart, char *position, struct hash *oldVars) /* If position and oldVars are non-NULL, and oldVars' position is different, add it to the cart * as lastPosition. This is called by cartHtmlShell{,WithHead} but not other cart openers; * it should be called after cartGetPosition or equivalent. */ { if (position != NULL && oldVars != NULL) { /* If db changed (e.g. QuickLift hop or hgGateway switch), the old position is from