[Building Sakai] Comprehensive list of tools with skin.default/neoprefix problems (was: Portal NEO Prefix)

[Steve Swinsburg]

Nice analysis David. We need Jira's for these so they can each be looked at. Maybe a parent task with sub tasks/linked tasks (where projects are not part of SAK) so they can all be tracked.

This will fix the tools so they work the proper way, we still need a resolution on the neo prefix ;)

cheers,
Steve

On 21/03/2013, at 2:08 AM, "Adams, David" <da1 at vt.edu> wrote:

> After seeing wasted another several person-hours (not mine, but my colleagues') yesterday and today on further confusion about what files need to be where for which setting of default.skin and discovering that the login tool is one of the violators of the protocol, I decided to do some research into the issue of which tools are doing it wrong. This is not to shame anyone or make any statement about what the priorities should be around this issue, but just for informational purposes:
> 
> From https://source.sakaiproject.org/svn/sakai/branches/sakai-trunk-all, here's the list of the projects with a user interface element that either look up the skin.default property or hardcode library/skin/default/ somewhere in their release:
> 
> * All of them
> 
> The only projects that don't use skin.default or assume that the relevant parts of the skin will be named "default" are the ones without a user interface, eg, jsf, velocity, master, textarea, warehouse, webservices, edu-services, deploy, etc. Ultimately of the 70 subdirectories in the checkout of sakai-trunk-all, 42 of them are doing something wrong related to the skin.
> 
> Of course, by far the most violations come from the included help HTML files, which hardcode the path to the default skin. Perhaps the help files are a reasonable exclusion. So if we exclude those, we get a slightly more manageable list of just 26 projects that are using skin.default or hardcoding library/skin/default. One of those is portal, so that's allowed. Kernel does some iffy stuff, but surely kernel can break the rules. Config obviously needs to mention skin.default. In reference, all of the default gateway HTML files hardcode library/skin/default, but you're supposed to touch those anyway, so let that one slide.
> 
> So there are just 23 left:
>  basiclti - 1 Java source file that hardcodes a reference to /library/skin/default/tool.css into the launch_presentation_css_url property it sends to ???
>  chat - 1 Java source file that retrieves the skin.default property and uses it improperly
>  entitybroker - retrieves skin.default property and provides it via an API to ??? which may not use it properly
>  linktool - a PHP demo file that hardcodes a reference to /library/skin/default/tool.css
>  message - 1 Java source file that retrieves the skin.default property and uses it improperly 
>  profile2 - 1 Java source file that retrieves the skin.default property and uses it improperly
>  rwiki - 1 Java source file that retrieves the skin.default property and uses it improperly
>  search - 1 static HTML file for testing purposes that hardcodes the paths to several static files in /library/skin/default/
>  sitestats - 1 Java source file that four times retrieves the skin.default property and uses it improperly
>  login - 1 Java source file that twice retrieves the skin.default property and uses it improperly, 1 other Java source file that twice retrieves the skin.default property along with portal.templates and portal.neoprefix and twice attempts to duplicate the portal logic surrounding its use
>  metaobj - 3 instances in two java files that get the default.skin property from ServerConfigurationService and use it improperly
>  podcasts - 2 JSP files with hardcoded references to /library/skin/default/tool.css
>  samigo - 1 RSF template and one .inc static file with hardcoded references to /library/skin/default/tool.css
>  citations - 2 velocity templates and one Java source file with hardcoded references to /library/skin/default/tool.css 
>  emailtemplateservice - 2 RSF templates with hardcoded references to /library/skin/default/tool.css, one more hardcoded to https://vula.uct.ac.za/library/skin/default/tool.css
>  gradebook - 4 RSF templates with hardcoded references to /library/skin/default/tool.css
>  site-manage - 4 RSF templates with hardcoded references to /library/skin/default/tool.css
>  osp - ignoring the ones for osp-portal and xsl-portal, 1 hardcoded reference to /library/skin/default/tool.css in osp/presentation
>  reset-pass - 5 RSF templates with hardcoded references to /library/skin/default/tool.css, 1 CSS file that refers to /library/skin/default/images/logo_inst.gif
>  mailsender - 8 RSF templates with hardcoded references to /library/skin/default/tool.css
>  polls - 8 RSF templates with hardcoded references to /library/skin/default/tool.css, four of which also hardcode links to https://vula.uct.ac.za/library/skin/default/tool.css
>  lessonbuilder - 45 RSF templates and 1 JSP file hardcode /library/skin/default/ -- usually tool.css, sometimes portal.css
> 
> If you've got sakai-trunk-all checked out, the command I used to find these (and the number of files with violations in each subdirectory) was:
> 
>  $ grep -rl skin.default . |grep -v help |grep -v /.svn/ |cut -d/ -f2 |sort |uniq -c |sort -bnr
> 
> Take out the "|grep -v help" part to get all the violations. Drop everything after "|grep -v /.svn/" to see a list of the files.
> 
> -dave
> 
> ________________________________________
> From: Steve Swinsburg [steve.swinsburg at gmail.com]
> Sent: Tuesday, March 19, 2013 7:24 PM
> To: Charles Severance
> Cc: Developers Sakai-Dev; Adams, David
> Subject: Re: [Building Sakai] Portal NEO Prefix
> 
> I'm still confused about what you are trying to argue for here Chuck.
> 
> Here are three things I think you might be saying:
> 
> Possibility 1: That skin names need to be aligned between 2.8 and 2.9+?. Ie default in 2.8, neo-default in 2.9. default-horiz in 2.8, neo-default-horiz in 2.9
> 
> I don't think it matters if the skin names that come with Sakai out of the box are aligned, they are completely different. And most people would create their own anyway. See also Possibility 3 below.
> 
> 
> Possibility 2: That skins from 2.8 will still work in the neoskin portal?
> 
> That isn't the case, skins from 2.8 need to be updated to work in neoskin templates.
> 
> Here is a real scenario of why all of this is confusing.
> 
> Suppose the following sakai.properties:
> 
> portal.templates=neoskin
> skin.default=someskin
> 
> Currently, the portal will change this to be neo-someskin. So really my skin needs to be called neo-someskin. So its against what I set, that skin doesn't exist and I get no skin. Confusing.
> 
> Then I realise my mistake and go and either change my skin to neo-someskin or just change the property to neo-someskin since 'neo-' isn't appended twice
> 
> So now I have:
> 
> portal.templates=neoskin
> skin.default=neo-someskin
> 
> And everything works.
> 
> Now, in a site, I want to have a custom skin, so I set the property to 'neo-someotherskin' (because I remembered the portal prefixing thing and my skin is named correctly). So I have this site rendering as neo-someotherskin, and other sites rendering as neo-someskin. Cool.
> 
> Now someone changes my site skin to someoldskin. The portal prefixes it to neo-someoldskin and it breaks. But I cant just change my skin name to neo-someold28skin because that skin is for the older portal and needs to be adjusted to work in the neo portal. So I cant just reuse my old skins without changing them.
> 
> The point here is that portal.templates is always respected. There is no changing of the templates to suit the skin. So ALL skins must be compatible with whatever templates you have set in sakai.properties.
> 
> Therefore this prefixing is confusing and unnecessary, and I still need to make new skins that are compatible.
> 
> 
> Possibility 3: That when people upgrade their skin site property will still work and be automatically prefixed.
> 
> So this would be the case above where someone had set 'someoldskin' as the site property. Then they upgrade Sakai and that site no longer has a skin because
> 
> portal.templates=neoskin
> 
> and the site property becomes 'neo-someoldskin', that skin doesn't exist, and we get a string of text down the screen with no CSS whatsoever. This happened to me yesterday.
> 
> If we did upgrade 'someoldskin' to 'somenewskin' and adjusted the site property, then it still wouldn't work since it would still be prefixed with neo- to become 'neo-somenewskin'. So people are now tied in to using the neo- prefix (or whatever other prefix they want, but only ONE prefix) because it gets prefixed.
> 
> 
> Again, the prefix is unnecessary. Why cant I just set 'somenewskin' as my site property and be done with it? Why should I be restricted in calling my skins 'prefix-someskinname'? Does anyone really care that 'mycoolskin' maps to 'neo-mycoolskin' and looks nice in a directory structure? I should be able to call my skins somecoolskin-for-29 and somecoolskin-for-210, or somecoolskin-for-29-v2 or whatever I wanted to.
> 
> regards,
> Steve
> 
> 
> On Wed, Mar 20, 2013 at 8:48 AM, Charles Severance <csev at umich.edu<mailto:csev at umich.edu>> wrote:
> 
> On Mar 19, 2013, at 12:02 AM, Steve Swinsburg wrote:
> 
> The issue here is the automatic prefixing of the skin names by the portal. This is not necessary for sites to choose their own skin if the skin is compatible with the set of portal templates that they are using. It *is* confusing, made evident by this thread.
> 
> I need a more tangible example.  If we have two templates and they have different skin requirements perhaps even to the level of how tool.css interacts with portal.css, then it would seem list we need parallel sets of each custom skin.  And the prefix allows this.
> 
> If I look in reference I see:
> 
> ./skin/default/tool.css
> ./skin/default-horiz/tool.css
> ./skin/examp-u/tool.css
> 
> ./skin/neo-default/tool.css
> ./skin/neo-default-horiz/tool.css
> ./skin/neo-examp-u/tool.css
> 
> It is 100% parallel.  Now you might argue that we should have used a folder structure.  That is cosmetic and would not remove the need to pre-pend something - it would have been a slash instead of a dash.
> 
> So give me a counter proposal that still allows 100% parallel tool.css and portal.css but somehow has no "prefix" or something other way to switch between sets of skins.  And remember the site properties that put these strings in before any notion of switching skin sets was created.  I would really like a proposal that does not require database upgrade scripts to be run when skins are changed - which is how I designed the above - to be conversion free and allowing a site to roll forward and back with nothing more and a prop change.
> 
> Also remember that files like default/tool.css are sitting in tons of vendor branches and I wanted not to move the old skin files just for the sake of prettiness and cause a bunch of angst in vendors branches.
> 
> I am sure our documentation can be improved and perhaps a folder structure for new skins rather than a file naming convention might feel more elegant.
> 
> I thin that the primary source of confusion is that for nearly a decade *nothing changed* in this folder - and in 2.9 we introduced parallel skin / template sets - and perhaps with less than sufficient announcements / documentation / transition guides.   That causes an instant "who moved my cheese" reaction of course.  But at this point unless I am completely missing something, adding something like a different folder structure (which does not eliminate the prefix) just to make it a little more elegant probably does not meet a cost/benefit analysis now that we have figured it out and are nicely fixing the tools (I thank you for that and the Wicket developers that want to use the rich editor thank you for that and Noah thanks you for that since now he does not have to fix the Wicket tools:) ).
> 
> I was thinking I should update that 2005 document to point out how to handle onload dat from the portal if the tool is living large with JQuery.  We should be clear that what is there is JavaScript suitable for onload or similar moment when page loading has been completed.  I should further say in that document that document ready is *preferred* over onload as it will give tools the best chance of getting frame height right.
> 
> /Chuck
> 
> P.S. I am bumming that the detail regarding these design choices is only getting good discussion a year after 2.9 came out :)  A lot of thought went into the choices but as we were all sprinting in our little corners trying to get 2.9.0 out the door - it was me and Gonzalo flipping coins on this one.  Whatever is bad on this design (if any) is almost certainly my fault :)