Views ctools integration broken because of issue with preview() [#1013170]

Recently I've update to the latest dev's of views & ctools.
Suddenly the former working view-panels didn't display anything - the underlying view's weren't even rendered.
After some digging I've came across the fact that a check for the $display_id in view::preview() was "failing".
The issue is identified but the solution isn't - is it a issue with the check in view::preview() or is it an issue how ctools views_content integration calls the method?
Atm. I go for the first choice since the condition seems to be broken. I've I got the idea behind the condition it should enable us to overwrite the current display mode for the preview by adding a parameter.
But atm. even if no parameter is set the display is changed - and of course to a illegal display.
Attached views patch changes this behaviour (also includes some trailing white-space removal) - the attached ctools patch shows where the call to preview comes from and how the call could be fixed.

Comment	File	Size	Author
#24	Chrysanthemum.jpg	49.01 KB	andreytsoy
#14	1013170-views-preview-method.patch	580 bytes	dawehner
#14
	ctools-views-integration.patch	649 bytes	das-peter

	views-preview-method.patch	823 bytes	das-peter

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

dawehner

German

CreditAttribution: dawehner commented 3 January 2011 at 10:50

You are right preview() doesn't work at the moment without adding the display.

The ctools patch has to be applied, that's sure.

The question for me is, do we support preview() to display the default display or not.

Comment #2

das-peter CreditAttribution: das-peter commented 3 January 2011 at 11:15

I'm still not sure if we really have to patch ctools, if the condition patch to preview is applied, since ctools set's already a proper view::current_display.
But maybe I just don't understand the idea behind the condition in the preview method. Thus here my understanding how it should be:
If no view::current_display is set or a $display_id is provided and it differs from view::current_display try to set view::current_display to $display_id.
Cases:

No view::current_display set, no $display_id provided:
Try to set display to NULL -> fails and leads to a return FALSE.
No view::current_display set, invalid $display_id provided:
Try to set display to invalid value -> fails and leads to a return FALSE.
No view::current_display set, valid $display_id provided:
Display is set to the value of $display_id.
view::current_display set, no $display_id provided:
Nothing changes.
view::current_display set, same $display_id provided:
Nothing changes.
view::current_display set, valid $display_id provided:
view::current_display is set to the new value.
view::current_display set, invalid $display_id provided:
Try to set view::current_display to invalid value -> fails and leads to a return FALSE.

Comment #3

das-peter CreditAttribution: das-peter commented 3 January 2011 at 13:12

As discussed in IRC it seems this issue was uncovered by fixing this issue #1001542: $view->set_display($display_id); returns TRUE for displays that do not exist.
Before this one was fixed, setting an empty $display_id didn't trigger the return false.
Thus the condition never failed and the default display was used. The changed condition should lead to the same result.

Comment #4

zuzu83 CreditAttribution: zuzu83 commented 3 January 2011 at 23:43

I have same probleme with 6x version

I applied this patch and everything works fine

@das-peter tank you very much

Comment #5

drupal a11y CreditAttribution: drupal a11y commented 4 January 2011 at 07:39

Are these patches already integrated into the latest dev of views?
I get errors when I try to run them:

patch < ctools-views-integration.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git views_content/plugins/content_types/views.inc views_content/plugins/content_types/views.inc
|index 472b608..16ea5f6 100644
|--- views_content/plugins/content_types/views.inc
|+++ views_content/plugins/content_types/views.inc
--------------------------
File to patch: ^[[A^[[B
: No such file or directory
Skip this patch? [y] y
Skipping patch.
1 out of 1 hunk ignored

mori$ patch < views-preview-method.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git includes/view.inc includes/view.inc
|index e792da5..f30e0f5 100644
|--- includes/view.inc
|+++ includes/view.inc
--------------------------
File to patch:

Comment #6

burlap CreditAttribution: burlap commented 4 January 2011 at 08:11

ctools-views-integration.patch is a patch against ctools and not views. And if you read the output of the patch command, you will see some suggestions how to use it. Try:
In views:
patch -p0 < views-preview-method.patch
In ctools:
patch -p0 < ctools-views-integration.patch

Comment #7

drupal a11y CreditAttribution: drupal a11y commented 4 January 2011 at 08:48

Thanks for the tipps, applied the patches but the problem was not solved. I´ll keep on testing.

Comment #8

das-peter CreditAttribution: das-peter commented 4 January 2011 at 11:08

@mori: Only the views patch is needed. The ctools patch was only attached to show from where the call to the method view:: preview() comes from and how another workaround could look like.
If you apply the views patch the ctools patch is superfluous (Nevertheless, it shouldn't have any negative impact).

Comment #9

kndr

he/him

Polish

Rybnik, Poland

CreditAttribution: kndr commented 4 January 2011 at 14:19

@das-peter: Thank you very much for your patch! It saved me much time. I've also checked mentioned patch at #3 http://drupal.org/node/1001542 and in my opinion, it should be immediately rolled back. This is the true reason of our troubles! I suspect, this incorrect patch could cause upredictable effects in many other functions - not only inside preview function.

Comment #10

dawehner

German

CreditAttribution: dawehner commented 4 January 2011 at 14:25

That's not really true: The original patch made sense.

Comment #11

kndr

he/him

Polish

Rybnik, Poland

CreditAttribution: kndr commented 4 January 2011 at 14:28

Sorry, but not for me. In my opinion it is commited too early.

Comment #12

dawehner

German

CreditAttribution: dawehner commented 4 January 2011 at 14:35

@kndr

Can you tell whether the first patch of this issue helps you to fix the issue?

Comment #13

kndr

he/him

Polish

Rybnik, Poland

CreditAttribution: kndr commented 4 January 2011 at 14:44

@dereine Yes, patch views-preview-method.patch works good and fix this issue.

Comment #14

dawehner

German

CreditAttribution: dawehner commented 4 January 2011 at 17:25

File	Size
1013170-views-preview-method.patch	580 bytes

Here is a patch for 6.x-3.x

This should apply for 6.x-2.x, too.

Comment #15

merlinofchaos CreditAttribution: merlinofchaos commented 4 January 2011 at 17:26

Can someone confirm for me whether or not the CTools patch is necessary?

Comment #16

sgabe CreditAttribution: sgabe commented 4 January 2011 at 17:32

I can confirm that the patch in #14 works for me on Drupal 6.20 with Panels 6.x-3.8, CTools 6.x-1.8 and Views 6.x-3.x-dev. I did not use any patch for CTools.

Comment #17

kndr

he/him

Polish

Rybnik, Poland

CreditAttribution: kndr commented 4 January 2011 at 17:34

@merlinofchaos For me, #14 is good. Patch for cTools is not neccessary.

Comment #18

deepbluesolutions CreditAttribution: deepbluesolutions commented 4 January 2011 at 18:28

hi Merlinofchaos,

just to answer your question re the patch on ctools, I just downloaded the new dev release of ctools, and found that I needed to reapply the patch in http://drupal.org/files/issues/ctools-views-integration.patch as the views disappeared from panels again, once the ctools patch was applied they reappeared ! so yes I would say it is needed for the 7.x branch. thankyou for an awesome module.

Comment #19

Renee S CreditAttribution: Renee S commented 4 January 2011 at 18:31

Confirmed, had this issue for the latest dev 6.xes, and this patch fixed it.

Comment #20

dabeast CreditAttribution: dabeast commented 4 January 2011 at 18:42

I can confirm that the patch in #14 works for me (Drupal 6.20, Panels 3.8, Panels 1.8, Views 3 dev).

Comment #21

dawehner

German

CreditAttribution: dawehner commented 4 January 2011 at 19:34

Status:

Needs review

» Fixed

Patch got commited.

Thanks to everyone who worked on this issue.

Comment #22

drupal a11y CreditAttribution: drupal a11y commented 5 January 2011 at 14:28

LOVE YOU BOYS !!! THANKS FOR FIXING cause this made me weird.
Now everything runs perfectly with Drupal 7 release 1.0 and
Chaos tool suite 7.x-1.x-dev (2011-Jan-04)
Entity API 7.x-1.0-beta6
Field group 7.x-1.x-dev (2010-Dec-31)
Panels 7.x-3.x-dev (2010-Dec-16)
Semantic Views 7.x-1.x-dev (2010-Oct-30)
TableField - CCK for Drupal 6 - Field API for Drupal 7 7.x-2.0-alpha5
Token 7.x-1.x-dev (2010-Dec-18)
Views 7.x-3.x-dev (2011-Jan-05)

The only thing I noticed is, that some settings in the content-types had changed, but this maybe was due to the import of the database exported from the rc2 version.

Comment #23

19 January 2011 at 14:30

Status:

Fixed

» Closed (fixed)

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

Comment #24

andreytsoy CreditAttribution: andreytsoy commented 18 February 2011 at 11:11

File	Size
Chrysanthemum.jpg	49.01 KB

I am not an experienced user. Help, I have not activated the module Views. Error: Ctools disabled.

I need to get some of these files in some folder?

Thank you!

Comment #25

davidogutu CreditAttribution: davidogutu commented 13 March 2011 at 01:33

Category:	bug	» support
Priority:	Normal	» Minor

yeah, you just need to go to the ctools project page http://drupal.org/project/ctools.
download and extract the module before copying it into sites/all/modules/

good luck