This one was
suggested by dkbit98.
(Thanks for kicking off the suggestions thread with an easy one; being able to quickly fix the first thing suggested makes me look badass, yo.) The area at the very top of most pages looks like this:
That area can be "collapsed" (by clicking on the minus icon next to the date/time) to save some vertical space:
The problem is that the setting isn't being saved/persisted, so you have to keep collapsing it again and again as you navigate the site. What's interesting is that it
is being persisted when you're not logged-in... That's weird, right? Why does it only remember to stay collapsed when you're
not logged-in?
It turns out that there are two ways in which this setting gets saved: When you're logged-in, it gets saved as a so-called "theme option" via a JavaScript function named
smf_setThemeOption that depends on a support endpoint (
/index.php?action=jsoption), and when you're not logged-in, it gets saved as a cookie. (At least, that's how it's supposed to work.)
One of those two ways is still working (the cookie way), and the other is currently broken (the theme option way). I can only hazard a guess as to why
smf_setThemeOption() no longer works, but I think what probably happened is that theymos disabled the support endpoint for it. Looking at the code for that endpoint, I can see why he might have done that (it's not a well-designed feature; it exposes too much attack surface relative to its usefulness).
So, I can see two ways to fix the problem:
Fix ASince the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not).
The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO).
--- baseline/Themes/default/index.template.php 2008-04-30 18:30:34.000000000 +0000
+++ modified/Themes/default/index.template.php 2024-07-18 02:47:24.000000000 +0000
@@ -45,24 +45,27 @@
/* The version this template/theme is for.
This should probably be the version of SMF it was created for. */
$settings['theme_version'] = '1.1';
/* Set a setting that tells the theme that it can render the tabs. */
$settings['use_tabs'] = true;
/* Use plain buttons - as oppossed to text buttons? */
$settings['use_buttons'] = true;
/* Show sticky and lock status seperate from topic icons? */
$settings['seperate_sticky_lock'] = true;
+
+ /* Always use cookies to shrink/expand the headers? (Rather than cookies when not logged-in, and theme options otherwise.) */
+ $settings['header_collapse_by_cookie'] = true;
}
// The main sub template above the content.
function template_main_above()
{
global $context, $settings, $options, $scripturl, $txt, $modSettings;
// Show right to left and the character set for ease of translating.
echo '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml"', $context['right_to_left'] ? ' dir="rtl"' : '', '><head>
<meta http-equiv="Content-Type" content="text/html; charset=', $context['character_set'], '" />
<meta name="description" content="', $context['page_title'], '" />', empty($context['robot_no_index']) ? '' : '
@@ -105,66 +108,66 @@
// If we're viewing a topic, these should be the previous and next topics, respectively.
if (!empty($context['current_topic']))
echo '
<link rel="prev" href="', $scripturl, '?topic=', $context['current_topic'], '.0;prev_next=prev" />
<link rel="next" href="', $scripturl, '?topic=', $context['current_topic'], '.0;prev_next=next" />';
// If we're in a board, or a topic for that matter, the index will be the board's index.
if (!empty($context['current_board']))
echo '
<link rel="index" href="' . $scripturl . '?board=' . $context['current_board'] . '.0" />';
// We'll have to use the cookie to remember the header...
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
{
$options['collapse_header'] = !empty($_COOKIE['upshrink']);
$options['collapse_header_ic'] = !empty($_COOKIE['upshrinkIC']);
}
// Output any remaining HTML headers. (from mods, maybe?)
echo $context['html_headers'], '
<script language="JavaScript" type="text/javascript"><!-- // --><![CDATA[
var current_header = ', empty($options['collapse_header']) ? 'false' : 'true', ';
function shrinkHeader(mode)
{';
// Guests don't have theme options!!
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
echo '
document.cookie = "upshrink=" + (mode ? 1 : 0);';
else
echo '
smf_setThemeOption("collapse_header", mode ? 1 : 0, null, "', $context['session_id'], '");';
echo '
document.getElementById("upshrink").src = smf_images_url + (mode ? "/upshrink2.gif" : "/upshrink.gif");
document.getElementById("upshrinkHeader").style.display = mode ? "none" : "";
document.getElementById("upshrinkHeader2").style.display = mode ? "none" : "";
current_header = mode;
}
// ]]></script>';
// the routine for the info center upshrink
echo '
<script language="JavaScript" type="text/javascript"><!-- // --><![CDATA[
var current_header_ic = ', empty($options['collapse_header_ic']) ? 'false' : 'true', ';
function shrinkHeaderIC(mode)
{';
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
echo '
document.cookie = "upshrinkIC=" + (mode ? 1 : 0);';
else
echo '
smf_setThemeOption("collapse_header_ic", mode ? 1 : 0, null, "', $context['session_id'], '");';
echo '
document.getElementById("upshrink_ic").src = smf_images_url + (mode ? "/expand.gif" : "/collapse.gif");
document.getElementById("upshrinkHeaderIC").style.display = mode ? "none" : "";
current_header_ic = mode;
Fix BThe other way to go would be to resurrect the disabled endpoint...
The advantage to fixing it this way is that the state would be properly persisted and would effectively be "remembered" no matter what. There's also a very small traffic-wise advantage to not using cookies for this. The disadvantage is that the forum's attack surface would increase (with the below repair: negligibly, in my estimation, but still a strict increase).
I've hardened the offending endpoint, and the result looks safe to me, but theymos should apply his mind carefully to the if-guard I've added to
SetJavaScript() before deciding whether it's sensible to restore the endpoint or not (especially considering that there's an alternative fix).
--- baseline/Sources/Themes.php 2013-01-18 15:35:17.000000000 +0000
+++ modified/Sources/Themes.php 2024-07-18 02:50:57.000000000 +0000
@@ -1181,24 +1181,28 @@
$context['sub_template'] = $settings['catch_action']['sub_template'];
}
// Set an option via javascript.
function SetJavaScript()
{
global $db_prefix, $ID_MEMBER, $settings, $user_info;
// Sorry, guests can't do this.
if ($user_info['is_guest'])
obExit(false);
+ // Best to enforce the low-consequence subset of functionality that this function actually/currently gets called for...
+ if (empty($_GET['var']) || !in_array($_GET['var'], ['collapse_header', 'collapse_header_ic']) || !isset($_GET['val']) || !in_array($_GET['val'], ['0', '1']) || isset($_GET['th']) || isset($_GET['id']))
+ obExit(false);
+
// Check the session id.
checkSession('get');
// This good-for-nothing pixel is being used to keep the session alive.
if (empty($_GET['var']) || !isset($_GET['val']))
redirectexit($settings['images_url'] . '/blank.gif');
$reservedVars = array(
'actual_theme_url',
'actual_images_url',
'base_theme_dir',
'base_theme_url',
(Though I've described the problem only with respect to the collapsible section at the top of the page, there's another one, the so-called "info center" at the bottom of the main page, that these fixes also address.)