@flamisz opened this Pull Request on March 2nd 2021 Contributor

Description:

fixes #17104

fixes #10578

Review

  • [x] Functional review done
  • [x] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [x] Security review done see checklist
  • [x] Code review done
  • [x] Tests were added if useful/possible
  • [x] Reviewed for breaking changes
  • [x] Developer changelog updated if needed
  • [x] Documentation added if needed
  • [x] Existing documentation updated if needed
@flamisz commented on March 3rd 2021 Contributor

@sgiehl yes, you are right. I haven't thought about that. Let's wait what are @tsteur's thoughts on this.

@flamisz commented on March 4th 2021 Contributor

@sgiehl @tsteur luckily our ajaxHelper.js collects all ajax requests into an array and we already have an abort method for it:
https://github.com/matomo-org/matomo/blob/5e2c5dce5cd88ba3872a72ae3e7675514e3a6098/plugins/Morpheus/javascripts/ajaxHelper.js#L44-L58

So I added one more line to call this method on every reporting page load. I tested it locally and it did the job.

@Findus23 commented on March 4th 2021 Member
@flamisz commented on March 4th 2021 Contributor

This might also be related to #10578

@Findus23 yes, there is a chance it solves that as well. I try to reproduce the other issue without the modifications and then with this PR.

Update: I see now, the other issue is different. In this PR we just abort the ajax requests when we change reporting page. The other issue happens with a full page reload: the new page starts loading but the browser still shows the original page, which has an error now, because the request are not loading anymore.
One solution for that one could be to delay that notification message a little bit, that time should be enough to the browser to load the new page.

@flamisz commented on March 5th 2021 Contributor

I updated the PR, because I find out where is that error on dashboard coming from #10578.
I added 2sec timeout for showing that error message. In this way, if the user stays on the page and an error occurs, will see it anyway, but if we clicking away from the page, the browser will have time to start loading the other page, before the error message shows up.

@diosmosis commented on March 5th 2021 Member

I haven't tested this yet, but for the second issue, would it also work to cancel ajax requests in the window's unload or beforeunload events? If that would work, the solution wouldn't depend on a timeout which might avoid edge cases where it takes a long time for the next page to start loading.

@flamisz commented on March 7th 2021 Contributor

Hi @diosmosis, I modified the PR. Instead of using a timeout, I'm checking if the xhrStatus is aborted. If it's aborted, the error is not shown.

This Pull Request was closed on March 14th 2021
Powered by GitHub Issue Mirror