Seems that Drupal.settings.views.ajaxViews has some deprecated selectors sometimes so one possible solution is prevent to use that selectors into ajax_view.js file.

This issue makes exposed filters with ajax not to work correctly.

Comments

CRZDEV created an issue. See original summary.

crzdev’s picture

Status: Active » Needs review
StatusFileSize
new480 bytes

Here is the patch that seems solve the problem. Please review. Thanks.
Any other solutions are welcome.

kevin_martin’s picture

This patch worked for me, thanks!

golddragon007’s picture

"Seems that Drupal.settings.views.ajaxViews has some deprecated selectors sometimes"
For me it happened when I loaded the page normally with a view, then I overwrite it with the same view with different DOM ID with ajax. (or loaded with ajax and overwrite it with ajax.)

This patch fix the error which I get if I do the same, which I wrote above, but because there's no cleanup for the old views data, basically it will create just a sort of "memory leak", because every time, when I get a new view from the server with ajax, the Drupal.settings.views.ajaxViews and the Drupal.views.instances will be bigger and bigger, but the old views data won't be deleted from there.

As I see in the views module, if you have a view already in the page you should generate with the same DOM ID that view, instead generating a totally new one OR trigger the RefreshView Handler, which will update the view's content in the proper way.

chris matthews’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

The patch in #2 to ajax_view.js does not apply to the latest views 7.x-3.x-dev.

Checking patch js/ajax_view.js...
error: while searching for:
  var selector = '.view-dom-id-' + settings.view_dom_id;
  this.$view = $(selector);

  // Retrieve the path to use for views' ajax.
  var ajax_path = Drupal.settings.views.ajax_path;

error: patch failed: js/ajax_view.js:26
error: js/ajax_view.js: patch does not apply
andrew answer’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new496 bytes

Patch rerolled.

tmin’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #6 applies cleanly in the latest stable version.

Since this is an issue that basically breaks the filters completely when the target selector is not found in the page, and the proposed patch reverts the view to functioning withtou ajax, it makes views "more forgiving" instead of completely breaking it.

As for the comment from @golddragon007 :

This patch fix the error which I get if I do the same, which I wrote above, but because there's no cleanup for the old views data, basically it will create just a sort of "memory leak", because every time, when I get a new view from the server with ajax, the Drupal.settings.views.ajaxViews and the Drupal.views.instances will be bigger and bigger, but the old views data won't be deleted from there.

I don't see how this specific patch has anything to do with that description. If that description is true, a separate issue needs to be created.

The only thing that the patch does is check if the ajax selector is valid in the current page and if it is not, it returns, effectively acting as a fallback to the "non-ajax functionality".

golddragon007’s picture

@tmin: reproduction: Open your browser console and ask down the variables which I mentioned (simply write it in the name and hit enter). You will see that always after every view load (if the page does not refresh, but i.e. you replace the view) it will just increase the item number inside those variables.
Besides that I had only this error "Uncaught TypeError: Cannot read property 'top' of undefined ajax_view.js:148" when I tried to attach a view to a custom page with a not proper way.

tmin’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new115.32 KB

Changing this issue back to "Needs review" till we sort the 'memory leak' part out.

@golddragon007 : I created a development environment just for this that consists of the following:

1) A very simple content type with only a title
2) A very simple view with AJAX enabled and an exposed filter that allows you to filter by title
3) I have applied the patch and did some sample ajax filtering operations

Attached are the screenshots of the variables that you mentioned.

Nothing seems out of the ordinary here.

In your original comment you mention:

because every time, when I get a new view from the server with ajax,

Can you please tell me how you are doing that so that I can reproduce it in a development environment?

golddragon007’s picture

This issue doesn't exist if we use the default view, nor the leak nor the exception, as far as I remember for this. We had this exception when we had a page with two buttons*. Both buttons load a view via custom AJAX call to a custom module where the response it the views generated by the views module. In this case, when we called the first time both views there was no problem. But when we called the second time any of the two views it failed with the above 'Uncaught TypeError: Cannot read property 'top' of undefined ajax_view.js:148' message. After I've applied the patch from the #2, this error message was gone, but I've noticed at debugging the two mentioned variables keeps growing on every AJAX load (I don't remember if it did at exposed filter too or not). Then I dug a bit deeper into the views system, then I noticed when I regenerate the views on the server side I need to provide the dom id of the view, which is the views_dom_id in your picture if I remember well. After I've provided the id there was no need anymore for this patch because I did not get this error message anymore. Also, we had no issues to use exposed filters on 'default' views which were generated by views by itself on by it's own as you tested. Also, I don't think it's needed for that case this patch, do you get an error message if you don't have the patch applied? Because I noticed this leak issue when I was on a view that had this error message.
I might try out to dig out an example at the weekend from that system if I will still find it. Unfortunately, the owner of this issue doesn't say exactly in which case he had this error message. So what you need:
- code which generates views on ajax call (generate two separately with two calls and overwrite the first with the second one), when you generate view don't provide the dom id!
- the generated views should have exposed filter (because filtering is also broken in this case)
- the generated views need to have AJAX enabled.

* According to my original comment enought one, if you can overwrite the original view by a newly generated one.

tmin’s picture

@golddragon007 : one of the cases where I actually noticed this issue is when in the view template file there is no: <div class="<?php print $classes; ?>"> which means that the .view-dom-id-[dom-id] class is nowhere to be found in the page and the error occurs.

With the proposed patch the views' filters basically fall-back to being applied with a regular form submission (and page reload). This makes me think that in my use case I wouldn't be able to see the variable bloating that you are talking about since there is an actual page reload happening.

I'll try to reproduce this by loading and unloading views with ajax and will update this issue if I'm successful.

curriedn’s picture

We just had the same.
Digging up we found out that we had 2 dom_id on the page and the one used by the ajax wasn't the one used for the view classes.

The issue came from our custom theme and some duplicated code which created 2 dom_id, I think that this issue isn't really an issue.

venkatesh rajan.j’s picture

The #6 patch worked for me.

Thanks @tmin, for your hint about class=" print $classes; "

rcodina’s picture

Patch on #6 worked for me too. I already had <div class="<?php print $classes; ?>"> on my views template. Only the patch fixed this issue in my case.

renatog’s picture

Status: Needs review » Reviewed & tested by the community

Patch #6 really worked here too without problems

  • RenatoG committed 3e0a531 on 7.x-3.x authored by CRZDEV
    Issue #2907263 by CRZDEV, Andrew Answer, tmin, golddragon007,...
renatog’s picture

Status: Reviewed & tested by the community » Fixed

Moved to the dev branch. Thank you so much everyone

damienmckenna’s picture

Status: Fixed » Closed (fixed)

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