Opened 13 months ago

Closed 12 months ago

Last modified 12 months ago

#3813 closed Bug (fixed)

Update JQuery & JQueryUI

Reported by: SteveG Owned by: SteveG
Priority: critical Milestone: 1.12 - The Great 1.x Backlog
Component: UI - UX (AngularJS, twig, less) Keywords:
Cc: Sensitive: no

Description (last modified by SteveG)

We need to do an update of JQuery and JQueryUI.

JQuery 1.7.2 -> 1.9.1
JQueryUI 1.8.22 -> 1.10.2

As the last update was quite a long time ago, there might be some code changes required...

Attachments (6)

Website selector style.png (8.0 KB) - added by EZdesign 13 months ago.
After searching, the default jQuery UI style shows up
keep url fragment broken.png (28.3 KB) - added by matt 13 months ago.
page following search big space.png (16.3 KB) - added by matt 13 months ago.
tooltip wrapping too early.png (23.4 KB) - added by matt 13 months ago.
chart before.png (25.1 KB) - added by EZdesign 13 months ago.
chart before (without line on the left)
chart after.png (28.6 KB) - added by EZdesign 13 months ago.
chart after (with line on the left)

Download all attachments as: .zip

Change History (105)

comment:2 Changed 13 months ago by SteveG (sgiehl)

The history plugin we are using doesn't work with new jquery version. Unfortunatly it is not maintained anymore (see http://tkyk.github.com/jquery-history-plugin/). I'll try to fix that plugin or look for something to use instead

comment:3 Changed 13 months ago by SteveG (sgiehl)

It might also be possible to remove the jquery.tooltip plugin. It is not maintained anymore and jquery ui has now also a tooltip widget included...

comment:4 Changed 13 months ago by SteveG (sgiehl)

We will also have to update jqplot. As the old version doesn't work with new jquery

comment:5 Changed 13 months ago by greg (gka)

FYI: I tried to upgrade while working on the map widget, but that opened a door to hell so I stopped it after two lost hours or so.

comment:6 Changed 13 months ago by SteveG (sgiehl)

In 5e59c5948d2d28213bd29201bedeac2d8159d304:

refs #3813 jquery, jqueryui & jqplot update + some fixes

comment:7 Changed 13 months ago by SteveG (sgiehl)

In ab52eb72aa0b3d56c1d8c2c25a51021b8997798e:

refs #3813 jqueryui css update + dialog fixes

comment:8 Changed 13 months ago by SteveG (sgiehl)

In 5667e914cec6efce5b7663ef881ce47e27b0fc21:

refs #3813 widget reload on changing column layout was broken

comment:9 Changed 13 months ago by SteveG (sgiehl)

In f3806a26b79dcc9b6157c14699558825e06e1eb6:

refs #3813 readding jQuery.browser plugin, as it has been removed in 1.9

comment:10 Changed 13 months ago by SteveG (sgiehl)

In b5ab89c24e653e413e1deb2e20cb76eef5a007e1:

refs #3813 css files in libs folder should always be included first, so they can be overwritten in piwik css as clean as possible

comment:11 Changed 13 months ago by SteveG (sgiehl)

In 237517a6ffdbe330e9daacd2bba909560473eac8:

refs #3813 use piwik tooltip styling for map tooltips

comment:12 Changed 13 months ago by SteveG (sgiehl)

In f61bbbccd64138fc1881c7abfffa1b4260205192:

refs #3813 use jqueryui tooltip plugin instead of piwiks selfmade one for datatables

comment:13 Changed 13 months ago by SteveG (sgiehl)

In ed557581ddd922b6aab93e947e15e6999efef80f:

refs #3813 use jqueryui tooltip plugin instead of piwiks selfmade one for transitions

comment:14 Changed 13 months ago by SteveG (sgiehl)

In 7c584814f25a496da8953ad9a772d6f970864be8:

refs #3813 use jqueryui tooltip plugin instead of piwiks selfmade one for overlay

comment:15 Changed 13 months ago by SteveG (sgiehl)

In 82de4cc60d1d385c12dcbe6799f4d3a7ada2ba6e:

refs #3813 use jqueryui tooltip plugin instead of piwiks selfmade one for jqplot graphs

comment:16 Changed 13 months ago by SteveG (sgiehl)

In b823598f1a8274a94f35021a36c071fef22a7912:

refs #3813 removing now unused piwik tooltip.js

comment:19 Changed 13 months ago by SteveG (sgiehl)

In f56307346e2ab68a47be17bec7f991b83740aab8:

refs #3813 fixed language selector: always use form to post data, as token_auth is required for users so setting a correct link is obsolete now

comment:20 Changed 13 months ago by EZdesign (BeezyT)

In 7fd5e006a9b0803c832523af1acfa403c8f44896:

refs #3813 polishing tooltip a little: using arial and same font size for title as table headings

comment:21 Changed 13 months ago by EZdesign (BeezyT)

Stefan, did you use fade-in for showing the tooltip on purpose? IMHO, it's not ideal in some cases.

For example:

  • When you open a row action popover, the tip stays there for a second and fades out quite slowly. This looks a little broken because it's above the popover background.
  • In Transitions, when an item on the left or right has multiple lines and you move the mouse from one line to the next, the tip fades out an in. (Tested in Chrome and Safari)
  • While fading, the tooltip moves not really fluidly. To me, that feels like you have to be careful not to break it when using the UI.
  • In Transitions, when you hover an item in the center, it says X% of all page views. When you leave the item and move your mose down, you might put your mouse on the tooltip while it's fading out. This prevents the next tooltip from showing. This happend to me a number of times and it took me a while to figure out that this is because I move my mouse over the fading tooltip and then stop moving. As a result, tooltips sometimes don't show up and it feels a little broken.

So I would propose not to use fade-in and all these items should be fixed. What do you think?

Is there a way to set the default options for $.toolip? Or would we have change every call?

comment:22 Changed 13 months ago by SteveG (sgiehl)

Currenty the default behaviour is used.
I don't think a global config is possible. I'll change every call and disable animations.

comment:23 Changed 13 months ago by SteveG (sgiehl)

In 6defcdabab75eb5365b77f375d0e07d45f513911:

refs #3813 disabled animations for tooltips

comment:24 Changed 13 months ago by SteveG (sgiehl)

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from new to closed

I'll close that ticket for now. If there are any regressions please reopen.

comment:25 Changed 13 months ago by matt (mattab)

  • Resolution fixed deleted
  • Status changed from closed to reopened

Superb commits... It is very useful to use these latest versions.

Feedback:

  • would it be possible to remove the jquery.browser.js? asking because the lib is deprecated
  • Maybe not a regression of this change, but on Widget view for a report, clicking "Row evolution" icon reloads the view rather than loading popover.
  • When a widget is maximised, clicking on the background should close the popover. Currently it does not. Clicking the "X" icon top right still works OK.
  • Note sure if it's a regression: Row Evolution popover eg. for Countries, or for Search engines Row Evolution, display the html code of the Icon, instead of displaying the icon in the popover title.

Great work anyway, works nicely.

Btw, are there other JS libraries, or JS plugins, we could update to their latest versions as well?

comment:26 Changed 13 months ago by SteveG (sgiehl)

I considered replacing jquery.browser, but I don't think it makes sense, so I readded it as a "plugin". jQuery suggests to replace it with one of those feature detection frameworks.
Also kartograph.js uses jquery.browser and I guess greg would have to change that there...

I'll have a look at your regressions and fix them asap.

I had a look at the other jquery plugins we are still using, but there aren't any newer versions.

comment:27 Changed 13 months ago by SteveG (sgiehl)

In 87d16aeb041d45f4f8144e9f1984d02120dd1095:

refs #3813 fixed maximised widgets not closing on click outside

comment:28 Changed 13 months ago by SteveG (sgiehl)

In 089b7df0d9ddeaa4c335380cdefdcb2231a51cb4:

refs #3813 allow html for popover dialog title

comment:29 Changed 13 months ago by matt (mattab)

I deployed these fixes on demo and it now works!

The last feedback would be to allow Row evolution and other RowActions in Widgetize mode, and maybe have a rule to enable them only if Width available > 700 or so.

comment:30 Changed 13 months ago by EZdesign (BeezyT)

Thanks for the updates, it's almost perfect now...

When a row action is clicked, the tooltip is not hidden immediately (i.e. in the click event). In Transitions, it is hidden once the popover content is loaded (which can be seconds after the click). For Row Evolution and Overlay, it is not hidden at all (at least as long as the mouse is not moved).
Could you hide the tooltip in the click event?

comment:31 Changed 13 months ago by SteveG (sgiehl)

@EZdesign: I can't reproduce that behavior. Which browser are u using?

comment:32 Changed 13 months ago by EZdesign (BeezyT)

It happens to me in Chrome and Safari. I didn't check other browsers.

comment:33 Changed 13 months ago by SteveG (sgiehl)

The tooltip of the rowaction is always hidden correct. But in a row with a row tooltip (truncated content) that tooltip sometimes shows up instead. I guess it's that what you mean? I'm not sure how to fix that, but I'll have a look.

comment:34 Changed 13 months ago by EZdesign (BeezyT)

Stefan, I just sent you an email with a screen capture of the issue.

Changed 13 months ago by EZdesign (BeezyT)

After searching, the default jQuery UI style shows up

comment:35 Changed 13 months ago by EZdesign (BeezyT)

I just noticed another detail (see attachment I just uploaded). I'm not entirely sure this belongs here but I think it didn't happen before.

Reproduce:

  • Open website selector
  • Search
  • Hover a search result

comment:36 Changed 13 months ago by SteveG (sgiehl)

I'll have look. Guess that happens because the new jQueryUI has new classnames at some points

comment:37 Changed 13 months ago by SteveG (sgiehl)

In 3e002e63d974df1f7949e9ae5aaf402cdeb46a7b:

refs #3813 fixed adding new websites/users. note: jQuery('htmlString') doesn't work anymore if htmlString doesn't start with '<' - use jQuery(jQuery.parseHTML('htmlString')) instead

comment:38 Changed 13 months ago by SteveG (sgiehl)

In 75382f806ff9c782836f3606f0c892f8fdf5572f:

refs #3813 fixed hover layout of site selector items

comment:39 Changed 13 months ago by matt (mattab)

Updated demo to beta5.

  • Click on black popover background does not close the popover. Reproduce: Click on "Give us feedback" then click on the background
  • The tooltip looks great.
  • However it is wrapped too early. It should display, when possible (up to a max width maybe 600px?), the text on one line. This is because the tooltip will be displayed often when the text is long, and it looks much better to show such text (often a URL or a keyword) in one line. See attached file showing issue.
  • on this report there is a funny space in "Pages Following a Site Search" (see attached file)


  • When adding a new website, "Keep URL fragment" is double HTML entitied (see screenshot)

Changed 13 months ago by EZdesign (BeezyT)

chart before (without line on the left)

Changed 13 months ago by EZdesign (BeezyT)

chart after (with line on the left)

comment:40 Changed 13 months ago by EZdesign (BeezyT)

I noticed a small but kinda ugly change in the evolution chart (see screenshots above).

jplot is now embedded as jqplot-custom.min.js. I can't debug the minified script. How can I build the minified script? Can Anthon or Stefan (who generated the script previously) place a README in the jqplot folder that explains the process?

The line is drawn on jqplot-grid-canvas, so it should be somewhere in jqplot.canvasGridRenderer.js. It's either a new parameter or a regression. I tried all documented parameters that seemed related but couldn't hide the line. I think we have to debug in the jqplot code.

Alternatively, we could revert the update. We don't depend on new jqplot features so using the old version would be OK - not ideal though.

comment:41 Changed 13 months ago by SteveG (sgiehl)

I don't know how anthon did that before. I had written a simple script that minified those files using the jsmin class.

Using the old version of jqplot didn't work with new jquery, that was the reason I upgraded it.

comment:42 Changed 13 months ago by SteveG (sgiehl)

@matt: I guess the url fragment thing got broken with your mass commit. There are whitspaces within the html tags. See https://github.com/piwik/piwik/blame/master/plugins/SitesManager/templates/SitesManager.tpl#L50
I'll fix that and the other regressions later...

comment:43 Changed 13 months ago by EZdesign (BeezyT)

Could you add the script to the jqplot folder so that we can just call it in the future to re-generate the minified file?

comment:44 Changed 13 months ago by halfdan

@sgiehl can you try and fix it as soon as possible - I will have to work on this template today (in my branch) and would like to avoid horrible merge conflicts with Smarty/Twig.

comment:45 Changed 13 months ago by EZdesign (BeezyT)

In c35534d42f4000a709b476c56526e886b24b0952:

refs #3813 removing the line that appeared on evolution and bar chars on the left after jqplot update

comment:46 Changed 13 months ago by EZdesign (BeezyT)

The problem is fixed now. A script to generate the minified file would still be nice :)

To be honest, I don't understand why we have to minfy it. Doesn't the addJs Hook do that?

comment:47 Changed 13 months ago by EZdesign (BeezyT)

This probably isn't the best way to do it, but what do you think about this script?

echo "" > jqplot-custom.min.js-temp

cat jqplot.core.js >> jqplot-custom.min.js-temp 
cat jqplot.linearAxisRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.axisTickRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.axisLabelRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.tableLegendRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.lineRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.linePattern.js >> jqplot-custom.min.js-temp
cat jqplot.markerRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.divTitleRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.canvasGridRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.shadowRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.shapeRenderer.js >> jqplot-custom.min.js-temp
cat jqplot.sprintf.js >> jqplot-custom.min.js-temp
cat jqplot.themeEngine.js >> jqplot-custom.min.js-temp
cat plugins/jqplot.pieRenderer.js >> jqplot-custom.min.js-temp
cat plugins/jqplot.barRenderer.js >> jqplot-custom.min.js-temp
cat plugins/jqplot.categoryAxisRenderer.js >> jqplot-custom.min.js-temp
cat plugins/jqplot.canvasTextRenderer.js >> jqplot-custom.min.js-temp
cat plugins/jqplot.canvasAxisTickRenderer.js >> jqplot-custom.min.js-temp

java -jar ../../js/yuicompressor-2.4.2/build/yuicompressor-2.4.2.jar --type js --line-break 1000 jqplot-custom.min.js-temp > jqplot-custom.min.js 

rm ./jqplot-custom.min.js-temp

Improvements / suggestions are welcome :)

comment:48 Changed 13 months ago by matt (mattab)

FYI: The reason the JQPlot JS were concatenated by Anthon was to make the JS Assets Merger much faster. Some web hosts were taking like 15-20seconds more to minify + merge these 19 files.

comment:49 Changed 13 months ago by SteveG (sgiehl)

In 3892944beee9698ae7b8cba50b7b4adecd007fe4:

refs #3813 fixes url fragments in sitemanager

comment:50 Changed 13 months ago by SteveG (sgiehl)

In e20f8ac0773e2d047e2928724308a6f7d62c0231:

refs #3813 set max tooltip width to 600px

comment:51 Changed 13 months ago by SteveG (sgiehl)

In 8b2f6711dbe18be7097cfbe1fd3f61c40cc19e0e:

refs #3813 rowevolution should work everywhere now

comment:52 Changed 13 months ago by matt (mattab)

In widgetize mode: https://demo.piwik.org/index.php?module=Widgetize&action=iframe&widget=1&moduleToWidgetize=Actions&actionToWidgetize=getPageUrls&idSite=3&period=day&date=yesterday&disableLink=1&widget=1

-> Row evolution is not working there. It adds the parameter &popover= to the URL instead of maybe creating the hash?

Note: If you fix this, you can also close #3570 which is to make Transitions+Row Evolution compatible with widgetize views :)

comment:53 Changed 13 months ago by EZdesign (BeezyT)

In b1db7def1cacdf04ab0c530da32cc46a53c2c7a8:

refs #3813 hiding row action tooltip when clicking the action

comment:54 Changed 13 months ago by EZdesign (BeezyT)

In 392161d93895ffaa270502581212de7251a4638a:

refs #3813 script for building jqplot-custom.min.js

comment:55 Changed 13 months ago by SteveG (sgiehl)

In a6c9cbae140bea006b7af29645ae35198c59a7a4:

fixes #3570 refs #3813 now rowactions should work everywhere ;)

comment:56 Changed 13 months ago by SteveG (sgiehl)

In 0047ff17d75b5e515cad115c4d95f18a0e0f0a7e:

refs #3813 fixed bug with ugly spaces in sitesearch datatable. problem was the datatable manager that didn't work with diffrent types of datatables on one page

comment:57 follow-up: Changed 13 months ago by SteveG (sgiehl)

In f3544da351ab5de4dce4d406a89d0a68b4317730:

refs #3813 don't show row actions if available window width is less then 600px

comment:58 Changed 13 months ago by SteveG (sgiehl)

  • Resolution set to fixed
  • Status changed from reopened to closed

I hope all regressions are fixed now. If not reopen ;)

comment:59 Changed 12 months ago by EZdesign (BeezyT)

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • In 0047ff17d75b5e515cad115c4d95f18a0e0f0a7e datatable_actions_js.tpl was removed. This file is still included in datatable_actions_recursive.tpl. Can we just remove the include or should the filename be changed to datatable_js.tpl?
  • After searching in a the pages report (search box at the bottom of the report), the data table JS does not work - neither sorting nor tooltips etc.
  • Also, search results with subtables are not shown properly, e.g. if you search for "index" here. They used to be nested like an expanded pages report.

comment:60 Changed 12 months ago by SteveG (sgiehl)

oh, guess I missed that one. Should be replaced with datatable_js.tpl
I'll have a look at that search issue asap

comment:61 Changed 12 months ago by EZdesign (BeezyT)

I'm not sure what the effect of changing the import would be, and it's hard to test as long as the other bug breaks the data table.
So I'm not comfortable with changing it, please take a look at it after the other bug is fixed.

comment:62 Changed 12 months ago by SteveG (sgiehl)

In 80f5e5bdec5fbaf62bb2cffde47759203bc478f7:

refs #3813 fixed template for recursive action tables

comment:63 Changed 12 months ago by SteveG (sgiehl)

@EZdesign: I guess that should fix your issues. Can you confirm, plz?

comment:64 Changed 12 months ago by matt (mattab)

We found that Overlay is not working anymore, eg: http://demo.piwik.org/index.php?module=Overlay&period=month&date=today&idSite=1#l=http$3A$2F$2Fpiwik.org$2F

  • Bug: Dropdown isn't showing selected period (however data on the right seems to apply to period)
  • Bug: The left panel shows "no data"
  • The right panel shows the bubbles in the websites as expected.
  • the particular page /download-piwik shows 12% next page in the transitions report but the "DOWNLOAD" menu link shows 0% for the same period.

comment:65 in reply to: ↑ 57 Changed 12 months ago by matt (mattab)

Row Actions work in the standard UI, but in the Iframed Dashboard, the icon hides on click but does not launch the popover.

comment:66 Changed 12 months ago by EZdesign (BeezyT)

In 2fbfbf76be47fbc8a9c8d33872f03ce8308cbff3:

refs #3813 another data table improvement: refactor data table header to a separate template

  • removes code duplication (we already had a minor inconsistent bug fix - shame on us!)
  • show the sorted column and report documentation after using the search box

@Stefan: the issues I reported are fixed now

@Everyone: this is turning into a general data table improvement ticket

comment:67 Changed 12 months ago by EZdesign (BeezyT)

In c00d063d46b88e8ec89313010012e387a375a4e0:

refs #3813 Overlay compatibility with latest jquery

comment:68 Changed 12 months ago by EZdesign (BeezyT)

Bug: The left panel shows "no data"

I don't understand. Can you clarify? When exactly does it happen?

the particular page /download-piwik shows 12% next page in the transitions report but the "DOWNLOAD" menu link shows 0% for the same period.

Is that connected to the select box showing the wrong period? Can you check again?
My guess is that you got confused about which range is shown because the select box didn't work. If the bug still exists, please tell us the exact period for both reports where the numbers don't match.

comment:69 follow-up: Changed 12 months ago by matt (mattab)

Nice refactoring Timo.

Regarding Overlay, now it's not loading any data for piwik.org: https://demo.piwik.org/index.php?module=Overlay&period=week&date=today&idSite=1

No data on the left and no bubbles in the right.

Overlay works OK on the forums& the crowdfunding site: http://demo.piwik.org/index.php?module=Overlay&period=month&date=today&idSite=7#l=http$3A$2F$2Fforum.piwik.org$2F

comment:70 Changed 12 months ago by matt (mattab)

Actually works on Chrome for piwik.org as well so looking good :)

IIRC the only small bug left to fix is:

  • Row Actions work in the standard UI, but in the ​Iframed Dashboard, the icon hides on click but does not launch the popover.
Last edited 12 months ago by matt (previous) (diff)

comment:71 in reply to: ↑ 69 Changed 12 months ago by SteveG (sgiehl)

Replying to matt:

Regarding Overlay, now it's not loading any data for piwik.org: https://demo.piwik.org/index.php?module=Overlay&period=week&date=today&idSite=1

No data on the left and no bubbles in the right.

Doesn't work with https for me, too. http is working well...

comment:72 Changed 12 months ago by EZdesign (BeezyT)

https://demo.piwik.org/index.php?module=Overlay&period=week&date=today&idSite=1

This works for me in Firefox, Safari and Chrome. Are you using exactly this URL? And which browser?
Please copy-paste the exact URL, tell me the browser and whether it's working or not. I'm a bit confused about what is working for whom in which case...

---

Row Actions work in the standard UI, but in the ​​Iframed Dashboard, the icon hides on click but does not launch the popover.

Stefan, will you handle that?

comment:73 Changed 12 months ago by SteveG (sgiehl)

In 30416f5a67aee234d38e3b203d8410affc2a3720:

refs #3813 fixes rowevolution not working in widgetized dashboard

comment:74 Changed 12 months ago by SteveG (sgiehl)

In ae2c2fd1cee3acdbee021210e44f9bbefa31826d:

refs #3813 clean solution to make popovers work in widgetize mode

comment:75 Changed 12 months ago by SteveG (sgiehl)

Is everything fixed now, or is something still open?

comment:76 follow-up: Changed 12 months ago by matt (mattab)

Nice updates!

Updated beta10 to demo and found some bugs:

  • the "Export as image" icon below evolution graph is not triggering the "Save As" box (does nothing)
  • on Search engines report, clicking "Make it flat" loads "no data".
  • on widgetize dashboard, clicking Row Evolution works, but it also displays "Loading Data..." below the "calendar". Instead it should not show this "Loading data..." since the popover loading message is enough.


comment:77 Changed 12 months ago by matt (mattab)

There was also a bug reported in the forums on IE8.

  • Using IE8, go to Actions> Page title report
  • Click on "Related report: Entry page titles" in the footer
  • error: "Message: Syntax error, unrecognized expression: The plugin undefined is not enabled. You can activate the plugin on Settings &gt; Plugins page in Piwik."

comment:79 Changed 12 months ago by SteveG (sgiehl)

In f2f4cd11fd637b727c68a38c02946d6ae508fb2f:

refs #3813 do not show a loading message if broadcast is initalized for popovers

comment:80 in reply to: ↑ 76 Changed 12 months ago by SteveG (sgiehl)

Replying to matt:

Seems to be related to the callbacks used by the api method of that datatable. Removing them makes the table work... don't know why -.-

  • on widgetize dashboard, clicking Row Evolution works, but it also displays "Loading Data..." below the "calendar". Instead it should not show this "Loading data..." since the popover loading message is enough.

fixed by my last commit

comment:82 Changed 12 months ago by SteveG (sgiehl)

As the search engines report seems to be fixed with [204cbc0af2cc2914235f6a720e4a5abf0d6b4101]. I guess everything is done?

comment:83 Changed 12 months ago by matt (mattab)

Nice fixes! I upgraded demo to b11.

I found a few more bugs:

  • Go to: here , click "all website" and the subtable loads "Google" but shows "1-25 of 102 Next ›" which is the paging info for the main table. Instead subtable paging should be "1-1 of 1" without next button.
    • similar/duplicate bug in search: Go to here search for "clear" below the subtable. On loading the search results, note that the paging information is wrong, and the "Next" button is displayed. Instead it should show "1-3 of 3" for the 3 search results, and hide the "next" button.
  • Row evolution regression: going to: dashboard, click on "Browser>Chrome row evolution" does not load the popover.

comment:84 Changed 12 months ago by matt (mattab)

Somehow the "Row evolution regressioN" I cannot replicate consistently.. sometimes it work, sometimes it doesn't.

Also tested dashboard and found:

  • Copying dashboard to user does not work. in the URL: user=undefined
  • Renaming dashboard also bugs

comment:85 Changed 12 months ago by SteveG (sgiehl)

In 0f80874acfd228070cb2a7335b4ad76d09cdbf06:

refs #3813 fixing dashboard regressions: jQuery.attr('value') might not return correct value anymore. use jQuery.val() instead!

comment:87 Changed 12 months ago by EZdesign (BeezyT)

In 8e95c379781dd65a97f33f9a58b9f35bcd7c7ac8:

refs #3813 fixing pagination after searching in a report

comment:88 Changed 12 months ago by SteveG (sgiehl)

In 235a231de94357003b833fb2514ec771a85457cd:

refs #3813 popover param wasn't removed in some cases; popover wasn't closed when using browsers back button on pages with empty hash part in url

comment:89 Changed 12 months ago by matt (mattab)

  • Priority changed from major to critical

The new Tooltip introduces XSS in Piwik!

Payload: piwik.php?idsite=1&rec=1&apiv=1&r=641368&_id=e1f7d8e178b7a866&3A42&fla=1&java=1&dir=0&qt=0&realp=0&pdf=0&wma=0&gears=0&ag=0&h=12&m=34&s=6&res=1024x768&cookie=1&url=http%3A%2F%2Fexample.org%2Findex.html&urlref=http%3A%2F%2Fthing1.com%2Fa%2Fb%2Fc.html%3Fa%3Db%26d%3Dc&action_name=second+page&urlref=http://example3.com/test%3E%22%27%3E%3Cscript%3Ealert%28%27XSS%27%29%3C/script%3E

Then go to Referers> Websites

I get a 'XSS' popup!

Luckyly I often use XSS payloads to test ;-)

Thanks for fixing!

comment:90 Changed 12 months ago by matt (mattab)

And nice work on the other fixes, appart from above, I can't find other problems for now :)

comment:91 Changed 12 months ago by SteveG (sgiehl)

I don't think that is caused by the tooltips, as they escape html by default. But I'll have a look...

comment:92 Changed 12 months ago by SteveG (sgiehl)

In ab74f9316a0e5c80f5099921f62fc6c1f832268c:

refs #3813 escape html title attribute to avoid possible XSS

comment:93 Changed 12 months ago by matt (mattab)

The bug is in the tooltip somehow (I think).

Fixing the datatable_cell is not enough as it appears in other cases as well:

piwik.php?action_name=%C3%83%E2%80%9E&idsite=1&rec=1&r=604235&h=11&m=3&s=38&url=http%3A%2F%2Flocalhost%2F&_id=c7200934de2aba4e&_idts=1364427469&_idvc=3&_idn=0&_refts=1366498880&_viewts=1365810845&_ref=http%3A%2F%2Freferrer.example.com%2Fpage%2Fsub%3Fquery%3Dtest%26test2%3Dtest3&cs=windows-1252&cvar=%7B%222%22%3A%5B%22%3Cscript%3Ealert(%27xss%27)%3B%3C%2Fscript%3E%22%2C%22%3Cscript%3Ealert(%27xss%27)%3B%3C%2Fscript%3E%22%5D%2C%221%22%3A%5B%22pagecvar%22%2C%22%20WOOOW%20%22%5D%7D&pdf=0&qt=0&realp=0&wma=0&dir=0&fla=1&java=0&gears=0&ag=0&cookie=1&res=1920x1080&generation_time_ms=23

Here it appears as a "Custom variable of page scope" in the Visitor Log, on hover on the page, it shows a XSS popup. (I could send few even more payloads for other reports).

So +1 for revert previous and let me know if you find a solution that would fix it globally (if not I can take a look!) thanks!

comment:94 Changed 12 months ago by SteveG (sgiehl)

In c1eb2001d421c7a5d8b083279f1058a8c4d6c2b2:

refs #3813 escape tooltip content for visitorlog to fix possible XSS

comment:95 Changed 12 months ago by matt (mattab)

  • When there is no data in the Visitor Log, the "Next" is displayed but it shouldn't be (since paging will not show data for the currently selected date)

comment:96 Changed 12 months ago by EZdesign (BeezyT)

"Next" is always shown in the visitor log. This is a hack that has been there forever. When I made the latest changes to the plugin, I made the hack better (i.e. less ugly) but it's still there. My point is that it's not related to this ticket and not even a regression. It could be added to a list of possible improvements for the visitor log.

comment:97 Changed 12 months ago by matt (mattab)

Thanks for clarification.

comment:98 Changed 12 months ago by SteveG (sgiehl)

  • Resolution set to fixed
  • Status changed from reopened to closed

Another try to close that ticket.
Hope there aren't any other regressions...

comment:99 Changed 12 months ago by matt (mattab)

Thanks Stefan. Sorry for silence last few days... I plan to do a bit more testing this week but code/features look very good!

Note: See TracTickets for help on using tickets.