This is jQuery XMLHttpRequest (jqXHR) object, not the native XMLHttpRequest object.

CommentFileSizeAuthor
#24 2496053-24.patch10.44 KBaj2r
#24 interdiff.txt399 bytesaj2r
ajax-jqxhr.patch1.09 KBdroplet
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,724 pass(es). View
#5 2496053-Rename-xmlhttprequest-to-jqXHR-in-Drupal-Aja.patch1.43 KBdarol100
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: failed to create checkout database. View
#9 2496053-Rename-xmlhttprequest-to-jqXHR-in-Drupal-Aja.patch1.54 KBdarol100
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,234 pass(es). View
#16 2496053-16.patch7.53 KBtameeshb
#20 interdiff.txt3.99 KBaj2r
#20 2496053-20.patch10.71 KBaj2r
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

darol100’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -JavaScript +JavaScrip

This patch seem good to me.

darol100’s picture

Issue tags: -JavaScrip +JavaScript

Fixing the issue tag.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
darol100’s picture

Assigned: Unassigned » darol100

I would work on this tonight.

darol100’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.43 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: failed to create checkout database. View

Here is the patch.

darol100’s picture

Assigned: darol100 » Unassigned
nod_’s picture

Status: Needs review » Needs work

The console.log needs to be removed and the JSDocs updated to match the new parameter name

core/misc/ajax.js
   391:8   error    Unexpected console statement                                 no-console
   391:26  error    Missing semicolon                                            semi
   618:2   warning  Expected JSDoc for 'jqXHR' but found 'xmlhttprequest'        valid-jsdoc

The last submitted patch, 5: 2496053-Rename-xmlhttprequest-to-jqXHR-in-Drupal-Aja.patch, failed testing.

darol100’s picture

Status: Needs work » Needs review
FileSize
1.54 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,234 pass(es). View

Do we need to updated the parameter on line 94 and 95 ?

Status: Needs review » Needs work

The last submitted patch, 9: 2496053-Rename-xmlhttprequest-to-jqXHR-in-Drupal-Aja.patch, failed testing.

Status: Needs work » Needs review

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

droplet’s picture

Status: Needs review » Needs work
Issue tags: +Novice, +js-novice, +Needs reroll, +DX (Developer Experience)

Please reroll with new fixing also :)

tameeshb’s picture

Status: Needs work » Needs review
FileSize
7.53 KB

Uploaded a fresh patch, please review!

chiranjeeb2410’s picture

Status: Needs review » Reviewed & tested by the community

@tameeshb,

Changes are good to go with. Changing to RTBC.

alexpott’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Needs work
+++ b/core/misc/ajax.js
@@ -110,8 +110,8 @@
-   * @param {XMLHttpRequest} xmlhttp
-   *   XMLHttpRequest object used for the failed request.
+   * @param {jqXHR} xmlhttp
+   *   jqXHR object used for the failed request.

How come we're not renaming the variable here? We do elsewhere in the patch. @droplet / @nod_ shall we rename all the variables?

+++ b/core/misc/ajax.js
@@ -745,12 +745,12 @@
+   * @param {jqXHR} jqXHR
    *   Native Ajax object.

@@ -938,14 +938,14 @@
+   * @param {object} jqXHR
+   *   Native jqXHR object.

I don't think the jquery UI is a native object. To me native object means something that exists a core feature of the javascript language. The point of this change is to document we're not using the native object :)

As a task this is only eligible for 8.4.x - you could argue that fixing the typehints and documentation is a bug. If we want to get this in 8.3.x then we need to separate up the bits that address the buggy documentation and task which is renaming variables to something better.

droplet’s picture

How come we're not renaming the variable here? We do elsewhere in the patch. @droplet / @nod_ shall we rename all the variables?

Yes, we can. It's better to read also.

the $.ajax() method returns the jqXHR object. 100% in Drupal Core is $.ajax calls I think.

aj2r’s picture

Hi,

I just changed the 'Native' and variable name.

aj2r’s picture

Issue tags: +DevDaysSeville

Adding tag.

aj2r’s picture

Status: Needs work » Needs review

The last submitted patch, 20: 2496053-20.patch, failed testing.

aj2r’s picture

Changed wrong line 424.

Jo Fitzgerald’s picture

Issue tags: -Needs reroll

Removed Needs Reroll tag.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.