New revs

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #180, on September 28th, 2010, 10:17 AM »
Quote
You mean the original mod had the hole
Sure did, and no-one ever called me out on it, either and the original mod was written back in December 2009...
Quote
Pwned! Didn't use insertQuoteFast
You can't use it to insert only parts of posts unless you modified it here...


I'll take care of the above momentarily.

:edit: Actually, I *did* take care of the permission checking for searching. Basically, the unset items are all to do with removing it from the list of things it knows about for searching/displaying within the manage area.
Posted: September 28th, 2010, 09:56 AM

Revision: 166
Author: arantor
Date: 09:16:59, 28 September 2010
Message:
! Ensure view-IP permissions are applied during install (install_2-0.sql)
! Add view-IP permissions to the internal templates (ManagePermissions.english.php)
& Move 'Logged' to Modlog (index.english.php, Modlog.english.php)
& Rephrase view-IP permission name and user help (ManagePermissions.english.php, Help.english.php)
----
Modified : /trunk/Sources/ManagePermissions.php
Modified : /trunk/Themes/default/languages/Help.english.php
Modified : /trunk/Themes/default/languages/ManagePermissions.english.php
Modified : /trunk/Themes/default/languages/Modlog.english.php
Modified : /trunk/Themes/default/languages/index.english.php
Modified : /trunk/other/install_2-0.sql
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,082
Re: New revs
« Reply #181, on September 28th, 2010, 11:35 AM »
Quote from Arantor on September 28th, 2010, 10:17 AM
Sure did, and no-one ever called me out on it, either and the original mod was written back in December 2009...
That's probably because we don't take code for granted, even our own.
Quote
You can't use it to insert only parts of posts unless you modified it here...
Ah yes, I had already merged it of course. Was faster to quote manually.
Quote
:edit: Actually, I *did* take care of the permission checking for searching. Basically, the unset items are all to do with removing it from the list of things it knows about for searching/displaying within the manage area.
Err, from what you're saying, your fix was already in your mod, then?
You've lost me here...
In my previous post, I mentioned how I'd found the fix in ManageMembers.php, with the unset().
Quote
! Ensure view-IP permissions are applied during install (install_2-0.sql)
So, in the end you're leaving the perms in, instead of using the ban perms?
Quote
& Rephrase view-IP permission name and user help (ManagePermissions.english.php, Help.english.php)
It still implies that can_view_ip_own doesn't exist, and that no one but the admin can have can_view_ip_any...
I guess it's a WIP for now :)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #182, on September 28th, 2010, 11:44 AM »
Quote
That's probably because we don't take code for granted, even our own.
I try not to, because I try to be mindful of the consequences of changing things.
Quote
Err, from what you're saying, your fix was already in your mod, then?
You've lost me here...
I initially thought I missed it, then I edited to say I hadn't, because of the unset items - which were in the original mod too. I missed your edit, it seems.
Quote
So, in the end you're leaving the perms in, instead of using the ban perms?
Yes... I see I wasn't clear with what I meant. I set it up so that users who would have ban permissions, would also gain by default the view-all-IPs permission in the templates and in installation, since if you have the ability to ban it follows that you probably should have view all IPs too.

But I didn't want to make them tied together; I can think of cases where users might want to give out some but not the others.

There's a thought, actually... ban user does also use the IP in the ban dialogue. I don't think I hit that one.
Quote
It still implies that can_view_ip_own doesn't exist, and that no one but the admin can have can_view_ip_any...
I guess it's a WIP for now
Yeah, I think so. I've found being a programmer makes me very good at some things - breaking down tasks and so on, but lousy in others - trying to communicate a complex concept to non-technical users (like with the help string) will tend to break down.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #183, on September 28th, 2010, 01:33 PM »
Quote from Arantor on September 28th, 2010, 11:44 AM
Quote
That's probably because we don't take code for granted, even our own.
I try not to, because I try to be mindful of the consequences of changing things.
You shouldn't worry about that too much. We're both capable programmers and we're both checking each other's code. Meaning that 90% of our errors are spotted by ourselves -- and the remaining 9% have a chance of being caught by the other. I'm leaving an extra 1% for future users to play with ;)
Quote
I initially thought I missed it, then I edited to say I hadn't, because of the unset items - which were in the original mod too. I missed your edit, it seems.
Yeah, we tend to edit too much :P
I also caught a message of yours on another topic, that you'd posted 2 minutes before me, and I didn't get the "Another message was posted" warning when sending it... I'm a bit surprised. Or maybe I just disregarded the message, I don't know. I'll have to answer it.
Quote
Yes... I see I wasn't clear with what I meant. I set it up so that users who would have ban permissions, would also gain by default the view-all-IPs permission in the templates and in installation, since if you have the ability to ban it follows that you probably should have view all IPs too.
Oh... Right. If it suits you then :)
I'm not sure exactly, but checking perms is a fast process, isn't it? I think that once perms are loaded for someone, they're cached.
Quote
There's a thought, actually... ban user does also use the IP in the ban dialogue. I don't think I hit that one.
Want me to look into all of the 'ip' occurrences in the code? (I think I might have better things to do today, though :P)
Quote
Yeah, I think so. I've found being a programmer makes me very good at some things - breaking down tasks and so on, but lousy in others - trying to communicate a complex concept to non-technical users (like with the help string) will tend to break down.
I'm not very good at that too. But I'm very good at bugging people :P Just make the text shorter and more vague! As if you were writing my résumé! :angel:

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #184, on September 28th, 2010, 02:10 PM »
Quote
I'm not sure exactly, but checking perms is a fast process, isn't it? I think that once perms are loaded for someone, they're cached.
It is, but it's not the fastest it could be (though the difference is basically a micro-optimisation) - $user_info['permissions'] becomes an array of strings of the permission names, matched via in_array. The faster way would be to array_flip it at some point during loading and do isset($user_info['permission'][$perm_name]), though note that if a board's id is specified, it's a DB query - and uncached IIRC. This is pretty infrequent however.

I'm quite happy to change it - it just seemed logical to tie it up into its own permission rather than tie it into something else; that way you don't give out permissions unnecessarily.
Quote
Want me to look into all of the 'ip' occurrences in the code?
Only if you get really bored.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #185, on September 29th, 2010, 10:14 AM »
Yeah, array_flip() could be beneficial. And instead of isset(), we could use !empty(), so we could actually set perm values to something else than just on/off: numbers, for instance... Like in 'max number of PMs allowed in your box', things like that. I'm not a perm specialist, though.

rev 167
* Updated to SMF rev 10144:
 ! Fixed missing global $smcFunc in prepareIndexes function (SearchAPI-Custom.php)
 ! $smcFunc['db_list_indexes'] was broken for SQLite. (DbPackages-sqlite.php)
 - removed getOuterHTML(), outdated javascript function (script.js) [Bug 4284]

- removed an extra getOuterHTML declaration which was not even used anymore... (repair_settings.php)
- removed getInnerHTML and setInnerHTML declarations, to encourage modders to use the proper way (innerHTML property.) (script.js)
- rewrote a couple of files that were still using getOuterHTML... Dunno if these files will be of any use one day anyway. (latest-packages.js, latest-themes.js)
! IE6 wasn't showing the hover color on menu items that had no submenu. (ie6.css)
* Spacinazi. (ManagePaid.template.php)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #186, on September 29th, 2010, 01:42 PM »
Quote
Yeah, array_flip() could be beneficial. And instead of isset(), we could use !empty(), so we could actually set perm values to something else than just on/off: numbers, for instance... Like in 'max number of PMs allowed in your box', things like that. I'm not a perm specialist, though.
I actually did something closer to this in SimpleDesk actually. Need to look at exactly what I did because I think I just had it store a value of 1, 0 or -1 depending on allow, disallow, deny (though they were constants instead so the code was clear)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #187, on September 29th, 2010, 02:23 PM »
Ahh...... You and your constants :D

I suspect the permission system overhaul[1] will be one of our biggest (and longest) subprojects.
 1. I loved how you mentioned an "overall overhaul" on arantor.org, ahah.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #188, on September 29th, 2010, 02:31 PM »
That's the thing... for me, seeing if ($some_array['value'] == ROLEPERM_ALLOW) is way more readable than if ($some_array['value'] == 1) - what does that 1 mean?

I also talked about magicalness; I have an idea for an easter egg out of it :D

Yeah, it will be one of the larger changes, but probably not the largest; some of the stuff I've suggested for group management is probably a touch larger.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #189, on September 29th, 2010, 03:23 PM »
Yeah it's more readable, I'm just saying -- you and your constants :P
It's not exactly of common use in SMF. Since I learned 90% of my PHP skills through the SMF codebase, I have to admit it's really not something I'd do myself. Not because it's no good -- only because I'm not used to that.
I'm only asking to be educated :)

rev 168 -- aka the IE6 mess fixer!
! Fixed header being too tall in IE6. (ie6.css)
! Fixed quick task list alignment in admin area under IE6. (ie6.css)
! Fixed contextual tabs hover state in admin area under IE6. (ie6.css)
! Fixed invisible gradients in cat_bar th declarations. (index.css)
* Replaced ie-css3 library with PIE, which is way bigger but also works perfectly in IE6/7/8. (PIE.htc, index.template.php)
* Reverted removal of get/setInnerHTML(), as they're currently being used in SMF's news files. (script.js)
Posted: September 29th, 2010, 03:19 PM

Re: IPs -- it's cool to have the icon in the left side now. Really... Thanks for the idea. Help text for IP: I didn't test, but should I assume that it only shows up if user HAS permission to see their own IP? If yes, then it's correctly phrased. Otherwise, it needs fixing.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs
« Reply #190, on September 29th, 2010, 03:53 PM »
Quote
Help text for IP: I didn't test, but should I assume that it only shows up if user HAS permission to see their own IP? If yes, then it's correctly phrased. Otherwise, it needs fixing.
Correct. If they can't see it, nothing shows at all.
Posted: September 29th, 2010, 03:27 PM

Revision: 169
Author: arantor
Date: 14:52:32, 29 September 2010
Message:
! Speculative fix for specifying PayPal language from subscriptions to checkout. Not overly happy about binding the list of known 'languages' into the core, but don't really see a better way at this time... short of including the language code into $txt in index.english.php (Subscriptions-PayPal.php)
----
Modified : /trunk/Sources/Subscriptions-PayPal.php


Yeah, it occurred to me as I was writing up the note that we could include the PayPal language code in the language file itself but didn't want to set that precedent (since if we include other payment gateways in the future, something similar would be required)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #191, on September 30th, 2010, 11:13 PM »
rev 170
+ Implemented the preceding space trick for smileys into IE. I think I love hating that crap they call a browser. (editor.js)

Phew! Let it be said that no day shall pass without at least ONE commit 8-)
Re: New revs
« Reply #192, on October 1st, 2010, 09:58 AM »
rev 171
* Renamed #footer_section to #footer, to match the earlier #header. (install.php, readme_*.html, ssi_examples.php, upgrade.php, index.template.php, index.css, ie6.css)
* Provided fallback border colors for browsers that don't support rgba() in CSS. (index.css)
* Rewrote buttonlist class to get better performance in IE and less clutter. Non-semantic spans are boring anyway. (index.css, index.template.php)
* Recompiled PIE library to shave 10KB off it. IE6 doesn't like it. Screw you, IE. Removed wrc and roundframe emulation for it. (PIE.htc)
! Fixed validation error in menus. (GenericMenu.template.php)
! IE9 doesn't need emulation code. For once... (index.template.php)

I'm currently considering dropping border-radius entirely for IE6... I realize I spent TWO fucking weeks on that piece of shit. Now I'm getting "Operation aborted" errors on PIE with it. Only happens SOME OF THE TIME. And of course it can't be debugged... That's the joy of HTC files. Maybe I should find a way to include the file differently (i.e. through JS, not HTC.)

MultiformeIngegno

  • Posts: 1,337
Re: New revs
« Reply #193, on October 1st, 2010, 10:07 AM »
:(

I'd say: don't give a shit about IE6! just... I'm really sorry you spent 2 weeks on this for nothing!

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs
« Reply #194, on October 1st, 2010, 10:34 AM »
Well, these 2 weeks were also spent on IE7 and IE8, which are way better and actually usable... (Except that IE7 also has Operation Aborted issues, but not with PIE, oddly.)