Deprecated synchronous XHR requests rewritten to async #8242

barbushin · 2015-06-29T14:51:48Z

It's the same as #8205 but excluding changes in /tests/javascript/index.php and fixed widgetMenu.js.

tsteur · 2015-06-29T22:17:01Z

plugins/Dashboard/javascripts/widgetMenu.js

-widgetsHelper.getAvailableWidgets = function () {
+widgetsHelper.getAvailableWidgets = function (callback) {
+    if(!callback) { // @deprecated
+        return this.getAvailableWidgetsSync();


I reckon we don't have to be backwards compatible here, it's not really a public API or so unless some of our code still uses it

tsteur · 2015-06-29T22:42:25Z

plugins/Morpheus/javascripts/ajaxHelper.js

@@ -74,6 +74,7 @@ function ajaxHelper() {
    /**
     * Should ajax request be asynchronous
     * @type {Boolean}
+     * @deprecated Synchronous XHR requests are deprecated(see http://xhr.spec.whatwg.org)


We should definitely mention this in https://github.com/piwik/piwik/blob/master/CHANGELOG.md under "Deprecations". We should also mention that we will remove it in Piwik 3.0. I created follow up issue #8246

tsteur · 2015-06-29T22:59:17Z

I tested them seems to work apart from one, nice!

Remove dashboard seems to not work anymore (Chrome 43 on Mac)

I have a few comments but they are not necessarily related to this PR, I'm not sure. We could create a follow up issue for this. Same of those issues might have existed before.

Eg when going to "Admin => General settings" and clicking "Save" (or Sites Management add/edit site or Users Management edit user) it could be nice to change the "state" of the button and use a wording like "Saving" and making the button disabled while the saving is in progress. We recently changed this in the Core Updater as one didn't know whether it is actually doing something or not. It was not really a big issue before as the requests were sync and one couldn't do anything while they were in progress
When clicking "Save" in "Admin => Plugin settings" it shows "Loading data..." above the "Save" button for a short time. When deleting a user it shows "Loading data..." for a short time as well. Also when pressing "Save" in Personal Settings.

I noticed some more problems but they seem not related to this change: #8245 #8246 #8247

tsteur · 2015-06-29T22:59:30Z

It's nice to have everything async!

barbushin · 2015-06-30T23:01:26Z

@tsteur Thank you for that review! I removed deprecated methods and notices. Also I checked #8245 and #8247 and all of them was reproduced on master, so it looks like they are not relative to this Pull request. Anyway, I submited pull requests with fixes for this issues.

tsteur · 2015-07-01T22:11:45Z

Sweet. Did you have a look re removing a dashboard? Does that work for you?
I will create issues for my other 2 points as they are a problem in general and not really relate to this PR

barbushin · 2015-07-02T07:13:44Z

Do you mean remove widget on Dashboard?
Looks like it works without any bugs.

tsteur · 2015-07-02T20:35:53Z

Sorry I meant removing an actual dashboard. This can be done in the "Dashboard & Widgets" selector in the bottom left.

barbushin · 2015-07-03T07:18:07Z

I've created new dashboard, and removed, and it works http://i.imgur.com/y9Hs1RF.gif

tsteur · 2015-07-05T21:42:38Z

For some reason it works for me too now. Cannot reproduce it anymore... Good to merge after 2.14.0 release

tsteur · 2015-07-06T01:10:02Z

Actually the Dashboard UI tests are failing, maybe because of this change?

mattab · 2015-07-12T22:45:53Z

Actually the Dashboard UI tests are failing, maybe because of this change?

I've restarted the build to check this can be reproduced. If the tests still fail then it's an issue either with the UI tests, or the PR?

barbushin · 2015-07-13T08:35:28Z

I don't undestand how to see/reproduce UI tests failure. I see links in UI tests results:

View UI failures (if any) here:
http://builds-artifacts.piwik.org/ui-tests.master/13864.7/
or: http://builds-artifacts.piwik.org/ui-tests.master/13864.7/screenshot-diffs/diffviewer.html

But it does not works.

mattab · 2015-07-13T09:35:08Z

@barbushin it looks like our screenshot uploader / UI test viewer is buggy for pull requests done in branches outside of Piwik/piwik (it works fine for PR done by core team members).
can you create a bug report here so we don't forget to fix this issue?

To workaround, could you try to re-create PR on the piwik/piwik branch and then maybe it works then?

Btw are you able to run UI tests locally and reproduce the UI test failure locally?

tsteur · 2015-07-13T12:36:31Z

FYI I created https://github.com/piwik/ui-tests-viewer/issues/11

It should be good to merge but can't see why tests fail. Should ideally only merge when tests are green.

barbushin · 2015-07-20T10:58:52Z

@mattab I re-created PR from piwik/8020_xhr_sync_fix branch. There are same UI Tests failing, but it looks like the problem is not in PR code change. Please take a loot at the comments there #8381 (comment)

mattab · 2015-08-13T10:27:34Z

Duplicate of #8381

Deprecated synchronous XHR requests rewritten to async

ec32784

barbushin mentioned this pull request Jun 29, 2015

Deprecated synchronous XHR requests rewritten to async #8205

Closed

tsteur added this to the 2.14.1 milestone Jun 29, 2015

tsteur added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Jun 29, 2015

tsteur reviewed Jun 29, 2015
View reviewed changes

This was referenced Jun 29, 2015

Adding/Editing a new goal doesn't update its "state" and some UI issues #8244

Closed

Mobile Messaging shows error message when changing setting #8245

Closed

tsteur reviewed Jun 29, 2015
View reviewed changes

tsteur mentioned this pull request Jun 29, 2015

Sites Manager doesn't save or show some saved global settings #8247

Closed

Remove deprecated async methods in widgetMenu.js

748f30c

tsteur mentioned this pull request Jul 1, 2015

Improve and fix behaviour / usability when saving entities (eg Users, Websites, ...) #8266

Open

mattab assigned barbushin Jul 12, 2015

mattab modified the milestones: 2.14.1, 2.15.0 Jul 16, 2015

mattab removed this from the 2.14.1 milestone Jul 16, 2015

barbushin mentioned this pull request Jul 20, 2015

Deprecated synchronous XHR requests rewritten to async #8381

Closed

mattab mentioned this pull request Aug 13, 2015

Remove possibility to send synchronous ajax requests #8246

Closed

mattab unassigned barbushin Aug 13, 2015

mattab closed this Aug 13, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecated synchronous XHR requests rewritten to async #8242

Deprecated synchronous XHR requests rewritten to async #8242

barbushin commented Jun 29, 2015

tsteur Jun 29, 2015

tsteur Jun 29, 2015

tsteur commented Jun 29, 2015

tsteur commented Jun 29, 2015

barbushin commented Jun 30, 2015

tsteur commented Jul 1, 2015

barbushin commented Jul 2, 2015

tsteur commented Jul 2, 2015

barbushin commented Jul 3, 2015

tsteur commented Jul 5, 2015

tsteur commented Jul 6, 2015

mattab commented Jul 12, 2015

barbushin commented Jul 13, 2015

mattab commented Jul 13, 2015

tsteur commented Jul 13, 2015

barbushin commented Jul 20, 2015

mattab commented Aug 13, 2015

Deprecated synchronous XHR requests rewritten to async #8242

Deprecated synchronous XHR requests rewritten to async #8242

Conversation

barbushin commented Jun 29, 2015

tsteur Jun 29, 2015

Choose a reason for hiding this comment

tsteur Jun 29, 2015

Choose a reason for hiding this comment

tsteur commented Jun 29, 2015

tsteur commented Jun 29, 2015

barbushin commented Jun 30, 2015

tsteur commented Jul 1, 2015

barbushin commented Jul 2, 2015

tsteur commented Jul 2, 2015

barbushin commented Jul 3, 2015

tsteur commented Jul 5, 2015

tsteur commented Jul 6, 2015

mattab commented Jul 12, 2015

barbushin commented Jul 13, 2015

mattab commented Jul 13, 2015

tsteur commented Jul 13, 2015

barbushin commented Jul 20, 2015

mattab commented Aug 13, 2015