The purpose of this module is to create a Drupal 6 module which exposes a permission 'holmes markup detective'. Assign this permission to a role on your Drupal site to a Role (e.g. "Content Editor", "Designer") and they will be warned of any potential accessibility problems during development and/or production. BTW, it's probably not a good idea to add this to the 'anonymous' role.

https://github.com/bbttxu/holmes

CommentFileSizeAuthor
#5 holmes-d6.zip140.28 KBbbttxu
#5 holmes-d7.zip42.82 KBbbttxu
#2 holmes.zip143.47 KBbbttxu

Comments

Everett Zufelt’s picture

Everett Zufelt commented
Status: Needs review » Needs work
Issue tags: +Accessibility

Thank you for submitting a module.

Please attach your module as a zip or tarball to this issue.

Also please provide a more thorough description of what problem your module attempts to solve, and how it is solved. Not being familiar with the module makes this all seem very mysterious to me.

Remember, you are asking volunteers to review your code, please make it as easy as you can for us to understand what you are trying to do with this module and to test it.

bbttxu’s picture

bbttxu commented
Status: Needs work » Needs review
StatusFileSize
new holmes.zip143.47 KB

Hi,

Sorry about that. I had pushed the changes to my d.o git account, but since the ticket is meant to make that public, forgot that no one could see it :)

"Holmes is stand-alone diagnostic CSS stylesheet that can highlight potentially invalid, inaccessible or erroneous HTML(5) markup by adding one class." http://www.red-root.com/sandbox/holmes/

Organizations need to ensure that their markup is valid and meets basic accessibility standards. This stylesheet offers that by visually displaying errors as they appear in a sites markup. A designer can use this stylesheet (along with other tools) to ensure that his/her theme's markup is valid before handing it off to the content editors. Content editors can be made aware of accessibility problems in new and existing content and fix while providing content to their sites.

However, bad content shouldn't be visible to anonymous visitors. This module provides a means to conditionally show the stylesheet on Role-based permissions; only people who are in positions to fix these problems should be able to see them.

The module is simple.

1. it provides a Permission that can be applied to a Role
2. The Holmes.css stylesheet is shown and a small pieced of JS is added (to 'trigger' the stylesheet) if a User is part of a Role that has that permission. Otherwise it is not included.

Everett Zufelt’s picture

Everett Zufelt commented

Other than there being a number of unnecessary files in the folder this looks pretty good. I would give this a +1. Would be nice (but not at all necessary) if there were also a Drupal 7 version of this module :)

+1

bbttxu’s picture

bbttxu commented

Thanks for the response. I can definitely see a D7 release soon.

As to the extraneous files that are present, they exist there since my original code is a fork of the holmes project, which contained those files. But as that project is simple, it won't be too much of a maintenance issue to just move the updated CSS file (and only that file) from there into this project.

—Adam K

bbttxu’s picture

bbttxu commented
StatusFileSize
new holmes-d7.zip42.82 KB
new holmes-d6.zip140.28 KB

Uploading cleaner versions for D6 and D7

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work

This seems like a great module. Can you create it as a sandbox project here on drupal.org so it can be more easily reviewed as you make changes?

bbttxu’s picture

bbttxu commented

I've created the project here: http://drupal.org/sandbox/bbttxu/1254448

Everett Zufelt’s picture

Everett Zufelt commented
Status: Needs work » Needs review

Setting back to needs review.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I noticed that this was forked from https://github.com/redroot/holmes which didn't have a license on it. It is now clearly licensed GPL. I didn't review any other code yet.

Everett Zufelt’s picture

Everett Zufelt commented

@bbttxu

Can you please confirm that this code can be released under GPL.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Everett - bbtxu has already done that by agreeing to the terms of service that allow him to create a sandbox on drupal.org. I just wanted to confirm that any upstream code that might have been included is also GPL'd. I think we're good on the licensing front, now on to style, code quality, functionality, security checks etc.

Everett Zufelt’s picture

Everett Zufelt commented 
function holmes_perm() {
  return array('holmes markup detective');
}

Might be a good idea to start the perm name with a verb 'Access', 'Use'.

From what I see in the module, if the user has the perm then the css is attached to the page, if not then it is not. Would it perhaps be useful to provide a toggle for this that doesn't require toggling a perm?

Otherwise code looks good. Still some unnecessary files in the git checkout.

bbttxu’s picture

bbttxu commented

Hi Everett,

Thanks for the feedback. I've pushed an update that expands the permission name from 'holmes markup detective' and 'show holmes markup detective' to indicate better what the module will do once toggled. I've also removed the README file as well. Aside from that file, not sure what other extraneous files are out there, but it's possible what I'm seeing in my local repo doesn't reflect what is on the d.o. git server.

I decided to use Permissions because it is built-in and provides exactly what I need it to do. I created the module to provide better quality control for designers/content editors/administrators Roles when setting up, and then running a live production site and ideally it would be toggled on the Permissions for each Role.

Alternatively, a Role could be set up whose sole purpose is to provide this one permission and that Role (ex. "Holmes viewer") could be applied to any User who needs the functionality if it doesn't line up with any existing Roles or if there are anomalous Permission schemes (ex. for whatever reason only two of five content editors need be concerned with invalid markup they may introduce on a production site).

The use case I don't imagine, is providing this for all "anonymous" users, though it can be done.

Using Permissions leaves it up to the site admins to decide the best, and most appropriate way of implementing it.

Thanks,

—Adam K

mgifford’s picture

mgifford
he/him
Primary language English
commented

Needless to say, I think this module is a good idea:
http://openconcept.ca/blog/mgifford/adding-css-markup-detective-drupal

I've linked to this thread from a number of other discussions in the issue queue.

Everett Zufelt’s picture

Everett Zufelt commented

@bbttxu

Extra files:
.DS_Store
.gitignore
*.zip
holmes.min.css (holmes.css being added in .module)

Not required, but it might be nice in a future version to allow those with the Holmes perm to toggle the CSS on / off on their profile. I might want to use it sometimes, but to disable it others, but might not have acess to modify my own perms.

mgifford’s picture

mgifford
he/him
Primary language English
commented

+1 on Everett's suggestion about toggling on/off user permissions for this. Could be just done with cookies easily enough.

bbttxu’s picture

bbttxu commented

Hi Everett and mgifford,

I managed to remove those files from the version branches, but not the 'master' branch. I just pushed up an update which removes the extra stuff from master as well.

I'll have to look into a way of toggling the CSS stylesheet per user preference, and I feel like that would add to the complexity of the module (not that the current module is anything near complex, but it does rely only on things Drupal has built-in). I guess I imagined it as if you were interested in accessibility, you wouldn't really want it off at all—that is your site (theme/content) would end up being perfect and therefore not show you warnings because everything was golden.

Though, I can see wanting to disable it every now and again—any ideas on how to do that? Any existing modules off which I can base that functionality?

Thanks for the feedback :)

—Adam

Everett Zufelt’s picture

Everett Zufelt commented

@Adam

For 7.x

/**
 * Implements hook_form_FORM_ID_alter() for user_profile_form.
 */
function holmes_form_user_profile_form_alter(&$form, &$form_state, $form_id) {
  // Add a setting to $form
  // You can save as a vriable variable_set('holmes_' . $form_state['user']->uid, 1);
  // Or using a cookie using user_set_cookie(); and user_get_cookie();
}
mgifford’s picture

mgifford
he/him
Primary language English
commented

Also, just wondering. Have you run the module through Coder yet? It's definitely a good tool to ensure that the formatting is good. Highlights silly spacing issues, but also a lot of other best practices.

Everett Zufelt’s picture

Everett Zufelt commented

If the 6.x and 7.x branches pass coder module then I say this is RTBC. The toggle on the user profile would be a feature request.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

I didn't realize how short the module is... I quickly reviewed it and confirmed it matches coding standards.

Thanks for your contribution, bbttxu! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Everett Zufelt’s picture

Everett Zufelt commented

FYI: #1289346: Provide a toggle on user profile pages

mgifford’s picture

mgifford
he/him
Primary language English
commented

Thanks everyone! Great to see this get moved along. Sorry I wasn't able to help more.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Btw, here's the link to the approved project:

http://drupal.org/project/holmes