UPDATE 1: Drupal.ajax.element should be a HTMLElement. @see #6

-------

The Drupal.ajax.expired function in core/misc/ajax.js is not sufficiently defensive: Namely, it loops through object members calling a contains() without first checking that said method exists; with the result, that if custom code adds unexpected members it causes AJAX requests to fail with a message like this:

In Chrome:

Uncaught TypeError: Failed to execute 'contains' on 'Node': parameter 1 is not of type 'Node'.

In Firefox:

TypeError: Argument 1 of Node.contains does not implement interface Node.

Possibly related to #2673824: Views JS passing wrong type of object to Drupal.ajax

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TravisCarden created an issue. See original summary.

TravisCarden’s picture

Assigned: TravisCarden » Unassigned
Status: Active » Needs review
FileSize
534 bytes

Here's a patch that tests that adds the missing conditional test.

droplet’s picture

typeof document.body === Node

always returns `false`, doesn't it ?

TravisCarden’s picture

Oops. I think I meant this: document.body instanceof Node

If that still doesn't look right, please propose a correction. JavaScript isn't my strong suit, so I'm kind of mucking about past my depth here. :)

droplet’s picture

Issue tags: +JavaScript

Seems like similar to #2705327: Failed to execute 'contains' on 'Node'.

Can you share any code example / steps to help me dig into it quickly ? Thanks.

nod_’s picture

Our docs are pretty clear: ajax.element should be a HTMLElement, which inherits from Element which inherits from Node.

Curious to know what triggers this issue for you.

TravisCarden’s picture

Hey, @nod_! Long time no see. :)

My team encountered this with Automated Logout 8.x-1.0-rc1. See autlogout.js.. The offending code seems readily apparent, if I understand it properly.

I was also having a problem with Entity Browser AJAX that this patch seems to fix, though I've taken no time whatsoever to identify the actual source.

Since the code already has a couple of defensive checks, it doesn't seem inappropriate to go the rest of the way to preventing problems from misuse. If the docs are thus clear, perhaps we should go ahead and test that it's an HTMLElement. Or if the point is to prevent developer error, perhaps some kind of error should be thrown.

droplet’s picture

Project: Drupal core » Automated Logout
Version: 8.1.x-dev » 8.x-1.x-dev
Component: ajax system » Code
Issue summary: View changes
Related issues: +#2705327: Failed to execute 'contains' on 'Node'
FileSize
808 bytes
TravisCarden’s picture

Title: Drupal.ajax.expired not sufficiently defensive » Non-HTMLElement values for ajax.element cause AJAX errors

  • AjitS committed 6a48afc on 8.x-1.x authored by droplet
    Issue #2706577 by TravisCarden, droplet: Non-HTMLElement values for ajax...
AjitS’s picture

Status: Needs review » Fixed

Tested the patch, and it resolved a couple of other bugs too :)
Committed and pushed to 8.x-1.x

Status: Fixed » Closed (fixed)

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