Unless a 403 or 404 happens, all page hits are collected as a "flat" structure, and when the webmaster watches the "page titles" tab, he sees ALL his page, wich can be very difficult to read when you have a large site with several hundreds nodes.

Piwik's JS API supports the use of a "piwikTracker.setDocumentTitle()" option that allows to classify the hits with the use of a / separator between levels.
So, my idea is to use Drupal's breadcrumb as the site's structure that must be shown under the "Page titles" tab. Other method are possible but with breadcrumb it was easy to code, and if a site uses a breadcrumb that is already configured, this is probably a clear and pertinent way to classify hits.

So I coded this functionality and created a patch. If some people find it usefull and if it is stable enough, it would be nice to include it in the dev for testing, then maybe later in 1.2 (?)
It adds two checkboxes in the piwik admin form : one to activate this feature (unactivated by default), and another to trim level 0 of breadcrumb (usually homepage) for better readability.

Patch is made to be applied from the "piwik" module base directory with -p0 option.

Please let me know if you find it useful, if you find a bug, if you want to include it in the module etc ...

Thanks and sorry for my poor english,

Hervé

Comments

hass’s picture

Status: Needs review » Needs work

Code style issues, security issue and emailaddresses in code.

Hervé Saladin’s picture

Thanks for your response.

I'll remove the emailadresses, sorry about that.

For the code style and security issues, could you please explain just a little bit what are those issues so I can fix it ?

By the way, what do you think of the feature itself ?

hass’s picture

If this really works I like this results. I'm just thinking if this should become a default...

http://drupal.org/coding-standards

1. No tabs -> 2 spaces per tab
2. drupal_to_js() for everything that is printed to the page source code.
3. one blank after for/if/etc.
4. never use \' in translatable strings if not really required. In such a case use double quotes and single quotes in text. Don't be lazy, do not use abbreviations.
5. Remove <br/> from translable strings
6. I guess urlencode() for title is wrong way for piwik. May need to be drupal_to_js()
7. Use one space after a comma.

Hervé Saladin’s picture

StatusFileSize
new3.4 KB

OK, thanks for the feedback, and thanks for the link ... I didn't know about these coding standarts but now I'll try to fullfill it.
Here's a second patch, with the corrections you asked, except one :
For the title, using urlencode() is the only way I can get the server-side of Piwik to recognise the title and split it (for the hierarchy) correctly. As my sites are in french, I tried to add some non-english characters in title (such as é, ç, à ...) I also tried to add some slashes and backslashes ... with urlencoding my titles it works great, and piwik display it clean, with the correct hierarchy. I tried with drupal_to_js() and that produces some very dirty unreadable titles.
Anyway, I use drupal_to_js to output the final escaped/concatened string in the client js code, so I guess it should be safe.

hass’s picture

Version: 6.x-1.1 » 6.x-1.x-dev
Status: Needs work » Needs review

Can you elaborate a bit what you are doing in Piwik? First I thought users only need to enabled this new checkboxes and they are done. From your last comments it sounds users need to customize piwik. If this is the case, please describe step by step what need to be done. Is this something official supported or only your hack? I cannot remember issues with the 404 and 403 stuff...

drupal_to_js() creates proper escaped JS code. As the title is inside a JS function we need to do this or JS code may break. It may looks "strange" in the webpage code, but this is the correct way. I'm not sure if piwik themself may do something wrong if it's decoded back...

Not sure why the patch is "ignored" here, but it shouldn't for automated testing...

Status: Needs review » Needs work

The last submitted patch, piwik_add_breadcrumb_hierarchy_2.patch, failed testing.

Hervé Saladin’s picture

No, it does't need any hacking of Piwik, nor any modification.
This is a natively supported feature of Piwik. See : http://piwik.org/faq/how-to/#faq_62

Piwik has it's own way to read the page titles that are set in the client JS code, and it seems to need these titles to be encoded with urlencode() on client's side.
In my case, I urlencode each part of the title (each step of the breadcrumb) individually, implode it with / sperators, then I finally use drupal_to_js() to output the whole string, so I can't see any reason why it shouldn't be safe.
I'm sorry if what I say isn't clear : english isn't my natural language, so even if I try to do my best, it's difficult for me to explain. Please take a look at my code, there's only a few lines and I think you'll quickly get what I mean.

For the patch, I can't figure out what's wrong : it works fine on my dev server. I used the stable branch to writeb my code and produce the patch, do I need to use the dev one ?

hass’s picture

Patch must be developed on top of dev

Hervé Saladin’s picture

StatusFileSize
new3.09 KB

OK, here it is (I also set the feature as activated by default as you suggested).

hass’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev
Status: Needs work » Needs review
hass’s picture

After some testing with your patch I figured out there is a major bug. The main issue comes from breadcrumb function as it does not give is the current page. In the breadcrumb trail this may be correct, but we need to log the current page, too or you will see the hierarchy, but not the page that has been seen. Very bad.

I decided to clone and alter the code behind drupal_get_breadcrumb() and move this with the functionality we need into _piwik_get_hierarchy_titles(). This makes some stuff easier and remove some "hacks". So we really get the title and not a link and so on. Some code became much easier.

Here are two new patches.

hass’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

jakew’s picture

Question: is there some way, if I want to do this as specified at http://piwik.org/docs/javascript-tracking/#toc-customize-the-page-name-d... , to just do it in the custom javascript code in the normal configuration page? Right now it's not finding the object piwikTracker if I this in:

piwikTracker.setDocumentTitle(document.domain + "/" + document.title);

jakew’s picture

Answered my own question -

_paq.push(['setDocumentTitle', document.domain + "/" + document.title.split(" | ")[0]]);