Agreed, that was the purpose of the 4.6/4.7 administration module done for civicspace. (i.e. rearrange the menus to be more logical and provide a "home" page for admins) About half the work done in the administration module was to remap all the admin menus. The other half was to provide a configurable admin "Home" page. The first part, remapping menu items, should all go into core and the contribs. The second part might be useful as a base for an admin home page.
<br><br><br><br><div><span class="gmail_quote">On 5/3/06, <b class="gmail_sendername">drumm</b> <<a href="mailto:development@drupal.org">development@drupal.org</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Issue status update for<br><a href="http://drupal.org/node/31326">http://drupal.org/node/31326</a><br>Post a follow up:<br><a href="http://drupal.org/project/comments/add/31326">http://drupal.org/project/comments/add/31326
</a><br><br> Project: Drupal<br> Version: cvs<br> Component: system.module<br> Category: feature requests<br> Priority: normal<br> Assigned to: Anonymous<br> Reported by: der<br> Updated by: drumm
<br>-Status: patch (code needs review)<br>+Status: patch (code needs work)<br><br>I was able to replace the admin screen with a URL alias.<br><br><br>Since HEAD development is reopened, I think we should consider
<br>/incrementally/ changing the administration section to be something<br>more useful than logs and more organized than the menu. And<br>establishing standards for contrib modules to properly integrate their<br>items and for us to stay on track in core.
<br><br><br><br><br>drumm<br><br><br><br>Previous comments:<br>------------------------------------------------------------------------<br><br>Thu, 15 Sep 2005 17:50:52 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/system_9.patch">
http://drupal.org/files/issues/system_9.patch</a> (4.27 KB)<br><br>New feature suggestion:<br><br><br>I would like to suggest a couple of enhancments to administration<br>pages.<br><br><br>First, add a new general settings option to allow the user to specify
<br>the admin home page. Currently the watchdog log view is the default<br>home page.<br><br><br>Second, add a new control panel page that can be used as the default<br>admin start page.<br><br><br>I've attached a suggested patch to the CVS head version of the
<br>system.module that implements these two enhancements. The patch does<br>the following.<br><br><br>Adds a new "default admin page" text field to the general settings<br>group (just after the "default front page"). It also changes the admin
<br>menu entry to call a new function that examines the default "admin page"<br>variable and redirects to the appropriate path. For the control panel,<br>the patch adds a new function (system_admin_controlpanel). This
<br>function basically looks for the admin entry in the menu structure and<br>then iterates through the child menu items and builds a control panel<br>page using the path and title information returned from the menu<br>structure. For the control panel icons it looks for an icon file in the
<br>misc directory that matches the name of the menu item (e.g. user.png,<br>access.png etc) If it doesn't find an icon it uses a default. I've<br>also included some 48/48 icons that I borrowed from my<br>usr/share/icon/gnome directory of my local Linux box. I believe these
<br>are gnome GPL'ed icons but any 48/48 icons could be used.<br><br><br>This patch allows users who prefer the current functionality to keep it<br>as their default. It provides the option for others to make any admin<br>
page they want to be their admin "home page" including using a<br>graphical control panel. Module developers that add new admin menu<br>options can add their own custom icons for the control panel but they<br>are not "required" to do so as a default will be used if they don't
<br>provide one.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Thu, 15 Sep 2005 17:52:15 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/drupal_controlpanel_icons.zip">
http://drupal.org/files/issues/drupal_controlpanel_icons.zip</a> (39.09 KB)<br><br>48x48 icons attached<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 16 Sep 2005 14:30:16 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/system_2_0.patch">http://drupal.org/files/issues/system_2_0.patch</a> (4.5 KB)<br><br>updated patch. slightly changed the formatting of the control panel<br>page
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 00:37:24 +0000 : moggy<br><br>I quite like this. It certainly goes some way to making Drupal easier on<br>
the eyes for first time users.<br><br><br>I noticed there's a lot of hard coded style. Would this not be better<br>in a stylesheet and the code just containing classes and ids?<br><br><br>Also would a dropdown box of possible admin pages be better than trying
<br>to remember how to spell controlpanel (something I'm having trouble with<br>tonight ;-) )<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 13:02:08 +0000 : syllance
<br><br>Nice job :)<br><br><br>the default admin page is a nice feature, and the controlpanel is a<br>very good idea. This goes in the good direction to make Drupal more<br>user friendly.<br><br><br>i agree with the dropdown menu and stylesheet additions, but i already
<br>really appreciate the current version.<br><br><br>hope this will go into core soon, as it really makes the first admin<br>pages contact better :)<br><br><br>i don't mind scrolling through the nav menu and i rarely goes into
<br>admin without checking the logs, but that definitely will help me<br>converting my wife's site to drupal :p<br><br><br>thanks !<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Sat, 17 Sep 2005 16:09:43 +0000 : der<br><br>I'll move the styles off to a style sheet but I'm uncertain about<br>whether or not make make the default admin page a dropdown. It's an<br>easy change and it eliminates any typos by the user but it also
<br>restricts the user to a current visible admin menu item. If by chance<br>someone wanted to write their own admin start page (ie their own control<br>panel) they would have to hack the core system.module to do it.<br>
<br><br>Anyone else want to weigh in with their preference?<br><br><br>Also, anyone up for creating drupalized versions of the control panel<br>icons?<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Sat, 17 Sep 2005 16:16:15 +0000 : chx<br><br>Maybe just because I created it, I like my control panel module better.<br><br><br><br><br>------------------------------------------------------------------------<br><br>
Sat, 17 Sep 2005 16:25:30 +0000 : der<br><br>If I would have known there was a module I wouldn't have written this<br>patch although I do think this would be good for core Drupal as the<br>default.<br><br><br>Is your modules a recent creation? I don't see it on
<a href="http://drupal.org">drupal.org</a> nor<br>could I find in the contribs section of CVS (either the modules or<br>sandbox sections). I'm I missing something? Is it called something<br>different?<br><br><br><br><br>
------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 18:20:15 +0000 : syllance<br><br>chx control panel is located in his sandbox (cp.module), and thanks to<br>him for letting us know there's one :)
<br><br><br>it's working fine (tested on HEAD) and provide interesting concept for<br>navigating through the admin, instead of just add a frontend. the<br>settings part is the better one, listing all elements in the page makes
<br>quickly forget the standard menu.<br><br><br>der's one looks nice and provide a more immediate access to admin<br>pages, but the standard menu is still mandatory to access some elements<br>(dba or store settings for example, still needs to be accessed via left
<br>menu).<br><br><br>to be honest, i like them both :)<br><br><br>being able to change the default admin page is very nice, and icons<br>make admin looks a lot better, raising the Woman Acceptance Factor by a<br>huge amount (my wife loves it :). but i also really like the more in
<br>depth admin navigation offered by chx module.<br><br><br>mixing both on my head setup gives a nice result, so may be it would be<br>a good idea to join forces and mix the 2 panels.<br><br><br>chx, is your module somewhat official and scheduled to hit the core ?
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 19:24:11 +0000 : Amazon<br><br>Hi, I asked Karoly to create the control panel module. It important<br>that the control panel be tied into the navigation menu block so that
<br>it does not create inconsistent navigation.<br><br><br>The goal is to start a user experience grouping exercise to help the<br>community categorize administration tasks. The first thing that has<br>to happen is that administration must be considered a seperate
<br>situation from creating personal content in Drupal. To support this<br>seperation of personal tasks and administration tasks we broke out<br>administration to have a separate theme in the CivicSpace theme.<br><br><br>
Through research interviews into Drupal administration we need to<br>discover what the goals and tasks of administrators are. Some early<br>feedback is that site developers need modules to be evaluated on<br><a href="http://Drupal.org">
Drupal.org</a>. They have also indicated they need better and consistent<br>administration help ,with incontext list, which we have been adding.<br>We have also heard administrators want a quick overview. Any change to<br>
provide a control panel like overview must have a dashboard like<br>overview. You should assume that we will identify a list of 5-7 top<br>goals for administrators.<br><br><br>Once those those 5-7 top goals are identified we need to ensure
<br>administrators can acommplish tasks to achieve those goals. Some of<br>those tasks will completed by clicking through to icons or admin<br>interface links. Sub-goals will be accomplished by providing<br>effective navigation. We need to consider 4 types of navigation to be
<br>added to the control panel: Global navigation, local navigation,<br>contextual navigation, and situational navigation. Some of this<br>navigation can be accomplished through a theme or blocks. Some will<br>need to be in the page and some need to be added in otherways, such as
<br>interaction techniques.<br><br><br>Keep all this in mind when creating a control panel solution. It also<br>must respond to the fact that every site will have different<br>permissions set and different modules. This is a complex problem and
<br>it's going to research and experimentation to get it right. But this<br>is a big step in the right direction.<br><br><br>Cheers,<br>Kieran<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Sat, 17 Sep 2005 19:47:55 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/system.module_11.patch">http://drupal.org/files/issues/system.module_11.patch</a> (5.61 KB)<br><br>This has sparked a lot of good discussion. I've taken a look at
<br>cp.module and I think it really is complimentary with my proposed<br>patch. I've taken my patch one step further and provided the settings<br>icons in a collapsable group below the main control panel. This would<br>allow access to all admin functions without referring to the
<br>traditional menu.<br><br><br>Syllance, do you think this addresses the gap you saw in my solution?<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 20:33:10 +0000 : der
<br><br>Kieran, I'm glad to see some serious thought going into the user<br>experience for site administrators. I think separating the user and<br>admin sections of the site is critical. It will not only allow<br>extensive usability work to be done for site admins it will also make
<br>it much easier for theme creators as they will be able to focus on the<br>end user experience. I like the admin theme for civicspace. My only<br>dislike about the approach is that it's template driven which causes a
<br>sites template to be larger and more complex due to handling all the<br>logic for user and admin themes. I think a better approach may be to<br>use a module such as the sections module. This allows you to have<br>smaller templates focused on user or admin themes.
<br><br><br>I agree with your point about making sure the control panel ties into<br>them menu system. My approach uses the existing menu structure so<br>modules are added dynamically and all permissions are enforced.<br>
<br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 22:21:05 +0000 : chx<br><br><?php<br>function _system_adminpage () {<br> drupal_goto($path = variable_get('site_adminpage',
<br>'admin/controlpanel'), $query = NULL, $fragment = NULL);<br>}<br>?><br>no way. Use menu !$may_cache and set the path based on the variable.<br><br><br>Adding in-line style elements is also a no-go.<br><br><br>The code style is not kept. Never a space between full stop and
<br>apostrophe, always otherwise.<br><br><br>It's not $menuvisible but $menu_visible<br><br><br>I must be blind (it's 0:17am) but // Build the settings section of the<br>control panel does not seem to differ from the previous section. A
<br>function may be appropriate.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sat, 17 Sep 2005 22:39:40 +0000 : der<br><br>Thanks for the code feedback. I'm assuming the extracted style info
<br>would get patched to drupal.css?<br><br><br>Your right about the duplicate code. A function would make more sense,<br> I'll make the change.<br><br><br>I'm not sure about the comment to use "$may_cache" I've never used it
<br>before. What's it's purpose? How should I use it in this situation.<br>I'll dig around and see if I can figure it out but I thought you might<br>be able to give me a little insight.<br><br><br>Thanks!<br><br><br><br>
<br>------------------------------------------------------------------------<br><br>Sun, 18 Sep 2005 10:41:00 +0000 : syllance<br><br>der, i've checked the new version of your control panel, and it does<br>simplify access to the settings menu (although with a complex setup,
<br>the site config collapsable menu goes a lot more down than the menu,<br>but this a page and not a small menu so its still better)<br><br><br>however, there other case settings like, such as the ecommerce modules,<br>which provides a more complex menu tree (ex :
<br>store->settings->payment->adjustments, each with their own admin<br>pages). these are not taken into account by your panel, leaving the<br>menu mandatory.<br><br><br>chx control panel handles this nicely with page navigation. going
<br>through the store menu with chx panel is a real pleasure while its a<br>real pain with the menu. So i think the best would be to mix the two<br>panels, chx one handling page navigation, and yours providing a<br>frontend for basic admin pages, and collapsable site config items.
<br>instead of linking to the standard admin pages, icons might link to chx<br>nav pages where there are sub entries.<br><br><br>i'm running both today, and this already makes the admin really better,<br>but i'm often switching from der panel to chx one (for store), so mixing
<br>both would be, for me, a very nice solution :p<br><br><br>thanks for the good work :)<br><br><br><br><br>------------------------------------------------------------------------<br><br>Sun, 18 Sep 2005 12:54:55 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/control_panel.patch">http://drupal.org/files/issues/control_panel.patch</a> (5.61 KB)<br><br>Here's a new patch with the code reworked per chx's comments with the
<br>exception of his objection to my use of drupal_goto function. chx, I'm<br>probably missing something really obvious but I'm not understanding your<br>suggestion. Are you suggesting I use some menu function to do the
<br>redirect? If so, I could not find one that suits my needs. Or are you<br>suggesting to use the drupal_goto function in a different manner than I<br>am using it. Like I said, I'm probably missing something really<br>
obvious here but for the life of me I don't know what it is. Any<br>further advice would be greatly appreciated. The offending code below:<br><br><br><?php<br>function _system_adminpage () {<br> drupal_goto($path = variable_get('site_adminpage',
<br>'admin/controlpanel'), $query = NULL, $fragment = NULL);<br>}<br>?><br><br><br>------------------------------------------------------------------------<br><br>Mon, 19 Sep 2005 13:42:57 +0000 : syllance<br><br>i think there's a problem with your latest patch. it seems to display
<br>all icons as a vertical list. it may be a problem with my setup, but<br>i'm using basic themes that comes with head. the administer page is<br>thus very long, and this is not very useful. previous version was<br>better, imho :)
<br><br><br>thanks<br><br><br><br><br>------------------------------------------------------------------------<br><br>Mon, 19 Sep 2005 14:10:03 +0000 : der<br><br>which theme and which browser are you using?<br><br><br><br>
<br>------------------------------------------------------------------------<br><br>Mon, 19 Sep 2005 15:59:16 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/control_panel_0.patch">http://drupal.org/files/issues/control_panel_0.patch
</a> (5.32 KB)<br><br>I've tested this with IE and FireFox with each of the Drupal shipped<br>HEAD themes and I'm not seeing the issue you described. Are you sure<br>the patch applied correctly to drupal.css?<br><br><br>
Here's a slightly revised patch. It make the control panel icon group<br>collapsable but open by default.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Mon, 19 Sep 2005 18:09:22 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/control_panel_1.patch">http://drupal.org/files/issues/control_panel_1.patch</a> (5.64 KB)<br><br>Sorry for the excessive number of patches. I just realized that my
<br>previous patch had an issue with icon image file names if there were<br>admin and admin/settings menu items that were the same (eg admin/user<br>and admin/settings/user).<br><br><br>New patch attached to address.<br>
<br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 02:13:49 +0000 : syllance<br><br>the vertical thing was probably due to a problem with my drupal.css.<br>i've tested your new patch with a fresh update and the display is fine,
<br>but there's a bug.<br><br><br>the site configuration panel displays store settings items<br>(checkout,payment,shipping), and not the global settings one. this<br>might be due to the menus item having the same name 'settings',
<br>although not located at the same level (administer/settings, and<br>administer/store/settings). ready for another patch ? :)<br><br><br>your panel adds a nice look and enable default administer page setup.<br>but there's still a lot of menu access needed when other modules are
<br>setup (such as ecommerce), and i'm using it in addition to chx one to<br>provide a complete admin solution :)<br><br><br>chx one looks just like the theme you're using with no icons, but it<br>really help accelerating admin navigation. i'm wondering if integrating
<br>both to core wouldn't be the ultimate solution. your panel more targeted<br>at standard admin needs, and chx one helping users that deals with lots<br>of modules & settings.<br><br><br>thanks again<br><br><br><br>
<br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 02:19:54 +0000 : Amazon<br><br>Hi, can you guys post some screen shots as you go along for those of us<br>who don't want to keep installing new patches.
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 02:25:06 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/Control">http://drupal.org/files/issues/Control
</a> Panel.png (101.02 KB)<br><br>Sure, here's what the current version looks like with the civicspace<br>admin theme.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 04:22:36 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/control_panel_2.patch">http://drupal.org/files/issues/control_panel_2.patch</a> (5.48 KB)<br><br>Patch fixing the issue that syllance pointed out and it also fixes the
<br>obvious issue with the drupal_goto function arguments. duh!<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 19:02:41 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/system.module_12.patch">
http://drupal.org/files/issues/system.module_12.patch</a> (2.48 KB)<br><br>I've thought about this and I think it makes sense to break this<br>enhancement request up and provide the second request (Admin Control<br>Panel) as an add-on module. Therefore I've scaled this patch back to
<br>handling only the first enhancement request:<br><br><br>/Add a new general settings option to allow the user to specify the<br>admin home page. Currently the watchdog log view is the default home<br>page./<br><br><br>
<br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 19:12:41 +0000 : chx<br><br>print theme('page', foo) is not used in HEAD, it's return foo. This<br>approach now looks pretty good. I had another approach in my mind but
<br>this begins to look good. I'll test it later.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 19:21:39 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/system.module_13.patch">
http://drupal.org/files/issues/system.module_13.patch</a> (2.47 KB)<br><br>new patch per chx's comment<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 20 Sep 2005 21:43:37 +0000 : syllance
<br><br>patch works fine.<br><br><br>this is a small but useful feature, i think it should be added to core<br>while waiting for the reworked control panel module.<br><br><br>may be we could close the thread and switch to a new one for the
<br>upcoming module, as the comments start to grow (guilty, your honor :)<br><br><br>keep up your nice work Der :) your patch was nice, but i'm sure we'll<br>soon have a nice admin panel module. (and there is no doubt about it if
<br>chx says it begins to look good :)<br><br><br>don't know anything about the approach mentionned by chx, but i really<br>like the result of having der panel + chx one for navigation, this<br>really gives drupal an improved admin usability.
<br><br><br>thanksē :)<br><br><br><br><br>------------------------------------------------------------------------<br><br>Wed, 21 Sep 2005 04:52:45 +0000 : der<br><br>FYI control panel module posted at <a href="http://drupal.org/node/31806">
http://drupal.org/node/31806</a><br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 11 Oct 2005 02:28:47 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/system.module_14.patch">
http://drupal.org/files/issues/system.module_14.patch</a> (1.78 KB)<br><br>rerolled patch for current CVS HEAD<br><br><br><br><br>------------------------------------------------------------------------<br><br>Thu, 13 Oct 2005 21:07:41 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/system.module_14_0.patch">http://drupal.org/files/issues/system.module_14_0.patch</a> (1.79 KB)<br><br>rerolled again for HEAD due to forms API changes<br><br><br>
<br><br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 13:44:50 +0000 : <a href="mailto:killes@www.drop.org">killes@www.drop.org</a><br><br>We do allow to set a custom front page, so this change makes sense to
<br>me.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 14:13:02 +0000 : moshe weitzman<br><br>Patch looks OK except for one thing:<br><br><br> look at how
index.php handles the $return value of<br>menu_execute_active_handler(). We should do same here. Ideally, you<br>should refactor this handling into a new function and use it in both<br>places. Perhaps the function should be called
<br>menu_print_active_handler().<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 14:16:06 +0000 : Dries<br><br>Care to share a screenshot?<br><br><br><br>
<br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 14:45:19 +0000 : moshe weitzman<br><br>Dries - there is a screenshot in this issue. See<br><a href="http://drupal.org/files/issues/Control%20Panel.png">
http://drupal.org/files/issues/Control%20Panel.png</a><br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 21:03:43 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/default_admin.patch">
http://drupal.org/files/issues/default_admin.patch</a> (3.42 KB)<br><br>New patch with changes suggested by Moshe<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 28 Oct 2005 21:04:46 +0000 : der
<br><br>Attachment: <a href="http://drupal.org/files/issues/default_admin.png">http://drupal.org/files/issues/default_admin.png</a> (26.45 KB)<br><br>Screenshot of just the admin/settings UI change.<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Thu, 17 Nov 2005 14:51:21 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/default_admin_page.patch">http://drupal.org/files/issues/default_admin_page.patch</a> (3.42 KB)<br><br>rerolled again for latest HEAD
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 00:23:34 +0000 : chx<br><br>Huh! Instead of menu_print_active_handler(); a simple return<br>menu_execute_active_handler(); isn't enough?
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 02:53:29 +0000 : der<br><br>chx, I changed it at moshe's request (see his comments futher up the<br>post). I'm fine with doing it either way.
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 03:41:26 +0000 : Richard Archer<br><br>The only thing I can see that needs attention is the phpdoc on<br>
menu_print_active_handler(). This is the same as the first line of the<br>phpdoc for menu_execute_active_handler(). These functions do different<br>things so one of them must be wrong.<br><br><br>Perhaps the menu_print_active_handler phpdoc should mention displaying
<br>the page or an appropriate error message.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 04:24:29 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/default_admin_page2.patch">
http://drupal.org/files/issues/default_admin_page2.patch</a> (3.47 KB)<br><br>New patch with changed phpdoc comment. It now reads:<br><br><br> /**<br> * Execute the handler associated with the active menu item and<br> * return the page or an appropriate error message.
<br> */<br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 04:30:08 +0000 : Richard Archer<br><br>I like it!<br>+1<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Fri, 18 Nov 2005 05:09:32 +0000 : asimmonds<br><br>For me, this makes it too easy to break my admin pages. If I enter<br>something invalid into the default admin page textfield, Apache crashes<br>when I goto /admin next time. The values that I tried were 'admin',
<br>'admin/not-exist' and a empty value, all crashed Apache. As well, if I<br>set it to a page that generates a 404, then the administer menu is not<br>expanded, making it harder to get to /admin/settings.<br>Could someone please confirm wether I'm not the only one that has
<br>problems with this.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 05:15:50 +0000 : Richard Archer<br><br>You are right. Apache crashes if the page is not found.
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 05:51:38 +0000 : chx<br><br>Also to Moshe: we are returning the value of menu_execute_active_handler<br>
so why would we need to handle it ourselves? index.php will handle for<br>us if I am not mistaken.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 12:48:27 +0000 : moshe weitzman
<br><br>chx - you might be right about. the previous patch had duplicated code<br>between drupal_ite_offline() and index.php and thats what i want to<br>avoid. i think a better solution would be to remove the break; in the
<br>OFFLINE branch?<br><br><br><br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 15:09:12 +0000 : der<br><br>I wasn't able to get Apache to crash on my system but I do see a problem
<br>with entering a path like 'admin/not-exist'. This causes a looping<br>condition as the menu system will fall back to the first valid menu<br>item in the path. Which in this case is 'admin' which obviously is the<br>
same path that was called in the first place. One way to resolve this<br>would be to validate that the path entered maps to a valid menu item in<br>the menu array first BEFORE 'menu_set_active_item' is called. Another<br>
way to address this would be to change the 'textfield' on the<br>admin/settings page to a 'select' that contains only valid menu items<br>to be selected. The only downside I see to this is I think it would<br>require a new function similar to 'menu_parent_options' only it would
<br>have to return all valid/selectable menu items.<br><br><br>OR<br><br><br>We could go a much simpler route. The existing admin menu entery look<br>like this with the callback directly to a watchdog function.<br><br><br>
$items[] = array('path' => 'admin', 'title' => t('administer'),<br> 'access' => user_access('access administration pages'),<br> 'callback' => 'watchdog_overview',<br> 'weight' => 9);<br>How about just allowing the function to be overridden by a variable?
<br><br><br> $items[] = array('path' => 'admin', 'title' => t('administer'),<br> 'access' => user_access('access administration pages'),<br> 'callback' => <em>variable_get('site_adminpage',<br>'watchdog_overview'),</em>
<br> 'weight' => 9);<br>This is a simple one line change. The variable does not have to be<br>changable in core drupal admin/settings. And it give module developers<br>an easy way to override the default.<br><br><br>
<br><br>------------------------------------------------------------------------<br><br>Fri, 18 Nov 2005 15:11:30 +0000 : der<br><br>oops...ignore my <em></em> tags in my last post<br><br><br><br><br>------------------------------------------------------------------------
<br><br>Fri, 18 Nov 2005 18:06:18 +0000 : der<br><br>Attachment: <a href="http://drupal.org/files/issues/default_admin_variable.patch">http://drupal.org/files/issues/default_admin_variable.patch</a> (1.17 KB)<br><br>Here's a patch for the 'simpler' approach described in my last post.
<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 06 Dec 2005 11:49:46 +0000 : moshe weitzman<br><br>this apache crash problem needs fixing before this can be accepted.
<br><br><br>also, the most recent patch lacks a settings form field. not sure if<br>thats deliberate or not.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 06 Dec 2005 14:12:25 +0000 : der
<br><br>It was deliberate. See my posting on 11/18. I'm suggesting to scale<br>this patch back to only providing a variable for the now hard coded<br>function name of "watchdog_overview". I don't think it makes sense to
<br>provide a settings form field for this.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Mon, 02 Jan 2006 02:43:07 +0000 : der<br><br>Just checked and this patch still applies cleanly to the latest version
<br>of the system.module.<br><br><br><br><br>------------------------------------------------------------------------<br><br>Tue, 02 May 2006 22:29:01 +0000 : <a href="mailto:killes@www.drop.org">killes@www.drop.org</a><br>
<br>moving to cvs. I am also wondering if you couldn't replace the default<br>overview suign a module that hijacks the /admin path.<br><br><br><br><br></blockquote></div><br>