[SMF 2.0 RC4] small bug in package manager

[emanuele]

While I was working on a mod I discovered this small bug.
It doesn't affect the install or uninstall operations, it's just a "visual" problem, but it could be annoying if during uninstall there are operations to check for manual editing.

Please find attached a demo package that can be used to reproduce the bug.

In this pack there are 2 modifications to two files.
1) the first one is a modification of PersonalMessage.template.php:

Code (find) Select Expand


// Send, Preview, spellcheck buttons.


Code (add before) Select Expand


echo '
<p>this is just a demo</p>';
This modification can be applied also to the "core" theme.

2) The second one is on index.php:

Code (find) Select Expand


require_once($sourcedir . '/QueryString.php');


Code (add after) Select Expand


// just another silly demo



During the installation everything is ok.
But if you try to uninstall and you check the details (with "details" I mean click on the icon that open the popup with the description of what to search and what to do, I'm not sure if it is the correct word for it...sorry) of the "index.php" operation SMF will show you the change to be applied to PersonalMessage.template.php.

As far as I can understand the problem seems to be related to the way the "operation_key" are enumerated: during the installation all the possible changes (also to non-default themes) are counted creating an array $mod_actions similar to:
[1] => operation (1) to PersonalMessage.template.php in default theme
[2] => operation (1) to PersonalMessage.template.php in core theme
[3] => operation (2) to index.php
and the page with the list of changes is generated based on this array (i.e. operation_key=1, operation_key=2 and operation_key=3).
During the uninstall process, instead, the array $mod_actions generated resemble to:
[1] => operation (1) to PersonalMessage.template.php in default theme
[2] => operation (2) to index.php
and the page with the list of operations is built based on this array (i.e. operation_key=1 and operation_key=2).
Probably when the details of the operation are checked (i.e. when the popup is generated) the array is filled like during the installation and operation_key 2 is considered the change to PersonalMessage.template.php in core theme instead of the change in index.php.

I tested it in a fresh install of SMF2 RC4.

[IchBin™]

I'm not sure I followed what the problem you described is. I installed your package, checking the install details. Looked fine, and installed fine. Then upon uninstalling, I review the same information and everything seemed fine to me. I did not see a problem in the uninstall details as you seemed to describe. Or did I not understand?

[emanuele]

I forget to mention: do not install it in core theme.
Does it make any difference in your test?

[IchBin™]

I did both test with adding to core and not adding to core. No problems here.

[emanuele]

Just because I didn't do a very cleaver example: so when you do the uninstall (without having installed the "mod" for the core theme) and you click on the details for "index.php" operation you get:

Code (find) Select Expand


require_once($sourcedir . '/QueryString.php');
// just another silly demo


Code (replace) Select Expand


require_once($sourcedir . '/QueryString.php');

That's fine, probably I have a problem in my clean install (and Darknico too), I'll check.
Thank you!

[NanoSector]

Just because I didn't do a very cleaver example: so when you do the uninstall (without having installed the "mod" for the core theme) and you click on the details for "index.php" operation you get:

Code (find) Select Expand


require_once($sourcedir . '/QueryString.php');
// just another silly demo


Code (replace) Select Expand


require_once($sourcedir . '/QueryString.php');

That's fine, probably I have a problem in my clean install (and Darknico too), I'll check.
Thank you!

I don't see any problem with it.

Find this, replace that code with that.

[emanuele]

I don't see any problem with it.

Find this, replace that code with that.

I know, but that's not the point.
The point is was that I didn't see what I wrote (that is what I expected to see), but the change for PersonalMessage.template.php listed in the details for the operation on index.php.

[emanuele]

I know I'm biased on that one...

BTW I still have the problem here on a fresh RC5.

Please try with the package I attached to this post.
When during uninstall I inspect the details for the index.php edit I see:


while the actual edit to index.php is:

Code Select Expand

<file name="$boarddir/index.php">
<operation>
<search position="before"><![CDATA[
require_once($sourcedir . '/QueryString.php');]]></search>
<add><![CDATA[
// this is a modification to index.php
]]></add>
</operation>
</file>

[Joker™]

Is the way package manage works, see this code in Packages.template.php, it is used to make that windows appear when you click on the icon.

Code (Find:) Select Expand

<body>
<div class="padding windowbg">
<div class="cat_bar">
<h3 class="catbg">
', $txt['operation_find'], '<a href="javascript:void(0);" onclick="return smfSelectText(\'find_code\', true);" class="smalltext" style="font-weight: normal;">' . $txt['code_select'] . '</a>
</h3>
</div>
<div class="padding">
<code id="find_code" style="overflow: auto; max-height: 200px; white-space: pre;">', $context['operations']['position'] == 'end' ? '?&gt;' : $context['operations']['search'], '</code>
</div>
<div class="cat_bar">
<h3 class="catbg topmargin">
', $txt[$operation_text], '<a href="javascript:void(0);" onclick="return smfSelectText(\'replace_code\', true);" class="smalltext" style="font-weight: normal;">' . $txt['code_select'] . '</a>
</h3>
</div>
<div class="padding">
<code id="replace_code" style="overflow: auto; max-height: 200px; white-space: pre;">', $context['operations']['replace'], '</code>
</div>
</div>
</body>


And most probably code for it is generated by here, Sources\Packages.php

Code (Find:) Select Expand

// Ok lets get the content of the file.
$context['operations'] = array(
'search' => $smcFunc['htmlspecialchars']($mod_actions[$_REQUEST['operation_key']]['search_original'], ENT_QUOTES),
'replace' => $smcFunc['htmlspecialchars']($mod_actions[$_REQUEST['operation_key']]['replace_original'], ENT_QUOTES),
'position' => $mod_actions[$_REQUEST['operation_key']]['position'],
);

[mu271828]

I ran into the same problem.  (Edit:  In SMF 2.0 final.)

Beside each operation is a little icon-link that opens a popup displaying the details of the operation.  (New in SMF 2; very handy.)  However, when uninstalling, sometimes the details are horribly confused, showing the details of other seemingly random operations, or nothing at all.  (The actual uninstall would work; just the display of the details is affected.)

As the OP suggests, the problem has to do with themes.  In ViewOperations() in Packages.php, when uninstalling, all themes are sent to parseModification(); presumably, as elsewhere, only themes in which the package is installed should be sent.  With the following changes, everything seems to work correctly:

Code Select Expand


--- original/Themes/default/Packages.template.php 2011-06-06 13:06:37.000000000 -0400
+++ modified/Themes/default/Packages.template.php 2011-07-24 23:16:10.000000000 -0400
@@ -161,7 +161,7 @@
echo '
<tr class="windowbg', $alternate2 ? '' : '2', '">
<td width="0"></td>
- <td width="30" class="smalltext"><a href="' . $scripturl . '?action=admin;area=packages;sa=showoperations;operation_key=', $operation['operation_key'], ';package=', $_REQUEST['package'], ';filename=', $operation['filename'], ($operation['is_boardmod'] ? ';boardmod' : ''), (isset($_REQUEST['sa']) && $_REQUEST['sa'] == 'uninstall' ? ';reverse' : ''), '" onclick="return reqWin(this.href, 680, 400, false);"><img src="', $settings['default_images_url'], '/admin/package_ops.gif" alt="" /></a></td>
+ <td width="30" class="smalltext"><a href="' . $scripturl . '?action=admin;area=packages;sa=showoperations;operation_key=', $operation['operation_key'], ';package=', $_REQUEST['package'], ';install_id=', $context['install_id'], ';filename=', $operation['filename'], ($operation['is_boardmod'] ? ';boardmod' : ''), (isset($_REQUEST['sa']) && $_REQUEST['sa'] == 'uninstall' ? ';reverse' : ''), '" onclick="return reqWin(this.href, 680, 400, false);"><img src="', $settings['default_images_url'], '/admin/package_ops.gif" alt="" /></a></td>
<td width="30" class="smalltext">', $operation_num, '.</td>
<td width="23%" class="smalltext">', $txt[$operation_text], '</td>
<td width="50%" class="smalltext">', $operation['action'], '</td>
@@ -255,7 +255,7 @@
echo '
<tr class="windowbg', $alternate2 ? '' : '2', '">
<td width="0"></td>
- <td width="30" class="smalltext"><a href="' . $scripturl . '?action=admin;area=packages;sa=showoperations;operation_key=', $operation['operation_key'], ';package=', $_REQUEST['package'], ';filename=', $operation['filename'], ($operation['is_boardmod'] ? ';boardmod' : ''), (isset($_REQUEST['sa']) && $_REQUEST['sa'] == 'uninstall' ? ';reverse' : ''), '" onclick="return reqWin(this.href, 600, 400, false);"><img src="', $settings['default_images_url'], '/admin/package_ops.gif" alt="" /></a></td>
+ <td width="30" class="smalltext"><a href="' . $scripturl . '?action=admin;area=packages;sa=showoperations;operation_key=', $operation['operation_key'], ';package=', $_REQUEST['package'], ';install_id=', $context['install_id'], ';filename=', $operation['filename'], ($operation['is_boardmod'] ? ';boardmod' : ''), (isset($_REQUEST['sa']) && $_REQUEST['sa'] == 'uninstall' ? ';reverse' : ''), '" onclick="return reqWin(this.href, 600, 400, false);"><img src="', $settings['default_images_url'], '/admin/package_ops.gif" alt="" /></a></td>
<td width="30" class="smalltext">', $operation_num, '.</td>
<td width="23%" class="smalltext">', $txt[$operation_text], '</td>
<td width="50%" class="smalltext">', $operation['action'], '</td>


Code Select Expand


--- original/Sources/Packages.php 2011-06-04 22:29:00.000000000 -0400
+++ modified/Sources/Packages.php 2011-07-24 23:14:18.000000000 -0400
@@ -1508,6 +1508,26 @@
$theme_paths[$row['id_theme']][$row['variable']] = $row['value'];
$smcFunc['db_free_result']($request);

+ // If we're viewing uninstall operations, only consider themes that
+ // the package is actually installed into.
+ if (isset($_REQUEST['reverse'])) {
+ $request = $smcFunc['db_query']('', '
+ SELECT themes_installed
+ FROM {db_prefix}log_packages
+ WHERE id_install = {int:install_id}',
+ array(
+ 'install_id' => $_REQUEST['install_id'],
+ )
+ );
+
+ while ($row = $smcFunc['db_fetch_assoc']($request))
+ $old_themes = explode(',', $row['themes_installed']);
+
+ foreach ($theme_paths as $id => $data)
+ if ($id != 1 && !in_array($id, $old_themes))
+ unset($theme_paths[$id]);
+ }
+
// Boardmod?
if (isset($_REQUEST['boardmod']))
$mod_actions = parseBoardMod(@file_get_contents($boarddir . '/Packages/temp/' . $context['base_path'] . $_REQUEST['filename']), true, $reverse, $theme_paths);


[Illori]

any progress on this?

[emanuele]

* emanuele bumps this because he wants to fix it.

[Arantor]

I've included this in a slightly tweaked form in 2.1, thank you