this->original_value does not always exist in views_handler_field->render_text() (line 1118

I have this problem with my commerce views. I wonder if this is somehow tied to the problem I am having with links that use "destination" in commerce not working properly.

After pulling the latest dev today, I now have this problem too for any empty field. Observed on text and file (generic file) fields.

Patch based on #1. Fixed problem and field handler tests pass locally with it.

Oops wrong patch.

Of course, you could argue that after the change isset($this->original_value) will still fail, but this seems to actually be a quality of the patch, not a default, as it impacts behavior the least, whereas using property_exists($this, 'original_value') would be slightly less stealth.

thank you, works for me.

+++ b/handlers/views_handler_field.incundefined
@@ -1115,6 +1115,9 @@ If you would like to have the characters \'[\' and \']\' please use the html ent
+    if (!isset($this->original_value)) {
+      $this->original_value = NULL;

It would be helpful to explain in a short comment this this variable might not be instanciated ... Alternative what about documenting the variable on the class level?

Actually, although this patch is minimal and answers the need well, I'm not fond of it, as it essentially appears to cover up an error higher up in the view build process, but I did not see where original_value should have been set in the first place, although this is probably where the problem should be fixed, instead of just at the point of use of the value, precisely because there could be other points of use and fixing in one place won't fix the others. Maybe you could point out where this comes from normally ?

Caused by the changes in #1273486: “Empty Text” does not work when “Hide Rewriting if field is empty" option is selected..

This version is just as short but much cleaner IMHO.

#9 Seems to have it all sorted.

Cheers!

Not really, dawehner mentioned on IRC that the field handler is used on each row, so by only initializing originalValue at handler construction, a valid value for one row won't be reinitialized for the next and can still cause the original error if the field is empty on the next row. That really needs a test case to qualify the next patch.

So adding some tags

The patch in #9 resolved the error on my site.

Patches could not help!

patch in #9 worked for me

Pathed in 9 does not seem to ve working in latest dev

Working on a test.

Okay, here's a test and a fix.

The reason why my earlier patch was not good enough (AIUI) is the situation of a test for which the first result row for a field is not empty, but one of the later rows is empty, which needs two nodes, the first of which has a value on the tested field and the other does not. I do not think your test as currently done would catch this.

Feel free to expand the test coverage.

EDIT

I say this only because I'm not 100% sure myself what to really test for.

RTBC for this fix, as I just came across this error, did a pretty much similar fix, then remembered this issue :)

+++ b/handlers/views_handler_field.incundefined
@@ -1014,6 +1014,10 @@ If you would like to have the characters \'[\' and \']\' please use the html ent
+      // If there are no items, set the original value to NULL.
+      if (empty($raw_items)) {
+        $this->original_value = NULL;

What happened with the documentation on the class level?

Additional: I somehow share the comment of @fgm, because there some cases which aren't covered by the tests :(

dawehner, do you mean what's in the patch in #9?

Hello.

I saw the error in the previous dev version and the newest dev version (2012-May-17) when in Views: Format Slideshow => view page.

#19: views-1548240-19-combined.patch queued for re-testing.

Not sure this needs to be re tested again?

Has this or is this going to be rolled into a Dev release?

I need this patch, but I would like to know whether or not it's going to be in the regular module codebase -- as I am regularly updating Views to the latest Dev version as they are released.

It will be added when the issue is marked as fixed and it says its been committed.

Okay, thanks; can you clarify what that means? Do you mean that the patch needs further testing before it can be declared a fix for the issue? Or are there some other processes that need to occur before the issue is marked fixed?

Hi.

I also have this issue.

The combined.patch in #19 solved my problems.

Tks.

#19 fixed the error in my case

Assign to myself to write a bit more tests

Just wanted to comment and say the patch from #19 worked for me as well

Patch worked for me too. This really should be added quickly to the latest dev version, IMO.

I agree with KingSalibah, the patch should be added, otherwise each time views is updated (I use the dev version) the patch has to be applied.

thanks

also confirm #19 that it works with current dev from June 1st. This must be commited.

well, everyone agrees..

It's all good and well everyone saying "..Must be committed" etc.. But no one is honouring dawehners comment in #33, that more tests are needed. If people want this to be committed they should try and help with tests, rather than just going with the just do it now approach.

well I would like to help with "tests" but I am a newbie for what concerns the code of the modules.
I don't know about which kind of tests you are speaking and what is needed to do.
Practically I apply the dev module plus the patch and I test it on my website, and come back here to report that it has indeed fixed the issue I had.
Thanks for the module and patchs!

The failing patch in #19 contains a test for the broken behavior I was seeing. It proves that the passing patch in #19 fixes that particular bug. fgm and dawehner are suggesting that there are other cases in which this is broken, and that those cases also need tests.

Has this patch been commited yet?

The issue will be marked fixed, if/when this is committed.

Rerolled #19 + #9

Thanks for the new patch! Commmited it to both 7.x-3.x and 8.x

New tests are coming

Yay!

This patch shows that there is still another bug involved.

So, the test only patch didn't fail. So something is wrong.

isnt it just me, or hasnt both passed the test?

@webadpro both passed the test. Which is wrong.

A new test, without a fix, should fail. It proves that something is broken. Then, when combined with the fix, it should pass.

This means that either the test is flawed and isn't proving the bug exists, or that the bug just doesn't exist.

Oh, I hear you. :) Makes Sense.

Actually i'm somehow convinced now that there is another bug i don't understand yet.
$this->last_render will be get a reset on every row, so maybe we could just get in the test.

I am not sure if this is related, but occasionally, since using the latest dev version, I have been getting various notice errors, but it still populates the data underneath the error notices. I have some interesting views contextual filters which I have been playing with. Normally I figure these out. But I just got one this morning, so I'll see if I can explain it and then perhaps you can tell me if it is related.

I have a node type (let's call is "cars") - one node per car brand. I have a block visible on the sidebar which has a contextual filter saying if the node type = say "BMW" then countDistinct all BMW models in stock. Let's say there are 7. This is a link to a page for the stock. The link retakes the node ID and uses that in the contextual filter for the page, so it pulls just the list of BMWs in stock. The page lists the BMWs in stock. This all works. All using views (no special coding).

Then I have exposed filters. Now, when I choose and apply a filter I get a notice error for each item returned. The error I get is "Notice: Array to string conversion in views_handler_field->get_render_tokens() (line 1385 of /serverpath...../sites/all/modules/views/handlers/views_handler_field.inc)." I get this error for each item returned, once a filter is applied. Related? Is it possible that the brand type is getting lost, creating this error?

I see my issue was solved at http://drupal.org/node/1566770#comment-6053584

#19 seems to have resolved this for me.

I am confused, too. I did not apply this patch after my latest update, and error does not show up anymore.
Patches in #44, #47 contain extra tests, are they still relevant?

The main bug is fixed but i thought of another possible one, but as the patch failed the status is wrong.

I was able to replicate this bug, and found another. I didn't yet update the test, just posting what I have.

patch applies cleanly, any more movement on this?

switching status for test-bot and unassigning as it has been over a year now.