New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #60, on August 20th, 2011, 10:13 PM »
Well... I guess we can try.

Oh, I think I'm walking into your territory now. In the spirit of my latest commit. I'm planning to split the change theme page into its own menu entry. Are you okay with that?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #62, on August 20th, 2011, 10:30 PM »
Good. Because I could always add a shortcut to that somewhere but i prefer a menu link to begin with. :)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #63, on August 20th, 2011, 10:39 PM »
I do have one idea I haven't shared yet that I think you'll like but I'll demo it when I've got it working how I'd like.

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #64, on August 20th, 2011, 11:32 PM »
If it's about an Ajax-driven theme changer in the footer, I already have the idea but too lazy to implement it :P

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #65, on August 20th, 2011, 11:35 PM »
Nope, not that. I'm not using AJAX either in what I'm thinking about, though I suppose it could be added since it's in an area that does have JS available, but I don't really see it as being needed.

The real question for me is whether I make it for all relevant users or a per-user thingamabob, but that has other consequences to be fixed since it would then be related to a vulnerability we discussed recently.

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #66, on August 21st, 2011, 12:41 AM »
I don't know what you're talking about... :^^;:

Oh, that theme picker is a real bitch when it wants... First of all, SMF bug: it sets the $current_action to 'admin' if 'sa=pick' isn't set in the URL... Even though it uses sa=pick when setting global and default themes within the admin area -- I changed that to checking for 'u=' in the URL. If it's set and u is > 0, it means we're editing a profile.

Also, if the user chooses the default theme, it also uses the default skin, right...? id_theme is set to 0, and skin is set to... Well, it's set to 'skins'. Which is totally wrong if you set the default theme to use a different skin than what it uses by default... Argh.
I'll have to check every single use of 'skin' to make sure it doesn't screw that up... >_<
Posted: August 21st, 2011, 12:39 AM

Oh, and something I haven't time to look into... If you use ;area=permissions in the admin area (e.g. the main menu's Admin > Permissions shortcut), you get an index page as expected, but neither the contextual buttons not the page description are correctly output. If I had ;sa=index manually at the end, it works. Only, $_REQUEST['sa'] at this point is always 'index', even when not set... Where does the error come from? Maybe it's checking into $_GET instead? Can you have a look, Pete?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #67, on August 21st, 2011, 12:43 AM »
Quote
I don't know what you're talking about...
That's the point, it's a surprise :D Though it was something you said that made me think of it... You'll see soon enough!
Quote
Oh, that theme picker is a real bitch when it wants...
Yes, yes it is.
Quote
Where does the error come from? Maybe it's checking into $_GET instead? Can you have a look, Pete?
That applies to every single page in the admin panel. It's done for a while and I think it's a bug of ours (i.e. I can't reproduce it in SMF). I suspect the change to tab_data is responsible from a bit back, but I'll take a look shortly.
Posted: August 21st, 2011, 12:43 AM

Or I will perhaps in a bit longer since my WampServer configuration isn't working properly. Might be conflicting with my original Apache setup >_<
Re: New revs - Public comments
« Reply #68, on August 21st, 2011, 01:06 AM »
OK, so WampServer is set and running.

Interesting fact of the day, I have to run Wedge with persistent connections, otherwise it takes a second to connect before going any further. Since SMF does it, it really has to be a problem somewhere in the bowels of the configuration (especially as WampServer appears to be using the originally configured MySQL server, not the one it installed with :/)

But, interestingly, I may have found a random bug where it actually got unset for some reason and I have no idea why that is.

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,667
Re: New revs - Public comments
« Reply #69, on August 21st, 2011, 02:49 AM »
Well it probably is on your side, maybe a conflict or something, because the mysql connection has never lagged for me on WampServer 2.1.
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #70, on August 21st, 2011, 02:55 AM »
Quote
Well it probably is on your side, maybe a conflict or something, because the mysql connection has never lagged for me on WampServer 2.1.
Oh, I finally found out what it was. It's the fact that it lags looking up 'localhost' in DNS, since using 127.0.0.1 works just fine.

Meanwhile I appear to have permanently broken phpMyAdmin's ability to connect. This is awesome, especially because I have no fucking idea how it happened.

For reasons too long to get into, all my local stuff is run with a root user with a password. root@localhost, root@127.0.0.1 and root@% are all defined and valid users in the mysql table. SMF and Wedge connect just happily with any of the above settings.

But phpMyAdmin won't. At all.

If I give it root@localhost, no password, it fails - understandably. But if I tell it root@localhost, with the right password, it tells me it can't connect root@127.0.0.1 (using password: YES). Which, considering they have the same password, is interesting. (Yes: it's actively converting the domain name into an IP)

And root@127.0.0.1 doesn't work either, even though it has the same password as the other root accounts (just varying the hostname)

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #71, on August 21st, 2011, 09:10 AM »
Why don't you try and clean up the mysql/php configuration files from your system and your registry...?
Re: New revs - Public comments
« Reply #72, on August 21st, 2011, 09:37 AM »
I've found the thing that doesn't work correctly in the menu code.

Admin.php:645 calls createMenu().
Admin.php:704 calls the admin function from the long admin array.

$_REQUEST['sa'] is reset to the default in the list only from within the later admin function, so at this point, createMenu was already created, and $menu_context['current_subsection'] was set in that function. There is one final opportunity to fix current_subsection but it's still in Admin.php and before line 704 (and this is something you added but I doubt it has any influence here.)

I suppose one simple fix would be to check for $_REQUEST['sa'] beforehand, and if empty (and the area is known), get the first working item from the 'subsections' sub-array. And work from that... We'd just need to ensure they're in harmony with the corresponding default from each individual function. I guess. Or we could even specify the fallback subsection as a new array entry in each area...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #73, on August 22nd, 2011, 01:35 PM »
Quote
And work from that... We'd just need to ensure they're in harmony with the corresponding default from each individual function.
The way it has always operated (and I think this is quite valid) is that if a given menu item has subitems, the first valid subitem is the one displayed as a default - and I don't see any reason to change that behaviour, personally. (IOW, going to SMF's own behaviour, which we seem to have broken at some point :/)

Interestingly this behaviour is also present in the profile area where you hit an item with subitems, without the default tab being highlighted...
Posted: August 22nd, 2011, 01:25 PM

OK, fixed it, it was broken in r886 with related changes to the menu code removing unnecessary stuff around figuring out the first/last items.
Posted: August 22nd, 2011, 01:33 PM

Committed in r961.

Nao

  • Dadman with a boy
  • Posts: 16,063
Re: New revs - Public comments
« Reply #74, on August 30th, 2011, 01:05 AM »
Thanks for the fixes (install time skin var & credits title), I totally missed these.

I'm not 100% sure about the removal of admin confirmation though... Not that I don't get your point -- but I can see people complaining the option is no longer there, and having to tell them to use phpMyAdmin to change it... I dunno. I suppose it's worth doing it -- and if too many people complain, we can always re-introduce the admin setting...

Even less sure about the removal of the help icon saying why the password has to be entered again. Are you... sure it's related to the above...? I wouldn't say it is. :^^;: