Skip to content
Permalink
Browse files

Fix incorrect `__isset()`.

There was a missing $ before name.  Fixes issues
with dynamic properties not being handled correctly.

Fixes #2450
  • Loading branch information...
markstory committed Jan 10, 2012
1 parent b8598c5 commit d238d8c0bbf51580bdb5dbd86c912c5b85bdbe0d
Showing with 13 additions and 1 deletion.
  1. +12 −0 lib/Cake/Test/Case/View/ViewTest.php
  2. +1 −1 lib/Cake/View/View.php
@@ -1236,4 +1236,16 @@ public function testExtendElement() {
TEXT;
$this->assertEquals($expected, $content);
}
/**
* Test that setting arbitrary properties still works.
*
* @return void
*/
public function testPropertySetting() {
$this->assertFalse(isset($this->View->pageTitle));
$this->View->pageTitle = 'test';
$this->assertTrue(isset($this->View->pageTitle));
$this->assertEquals('test', $this->View->pageTitle);
}
}
@@ -796,7 +796,7 @@ public function __set($name, $value) {
* @return boolean
*/
public function __isset($name) {
return isset($this->name);

This comment has been minimized.

Copy link
@ceeram

ceeram Jan 12, 2012

Member

oops, my dumb mistake. however this only works for properties not needing magic __get()

doing:

$this->request->params['action'] = 'index';
isset ($this->action) will return false

now DebugKit toolbar doesnt show anymore because its doing if (empty($this->output)) check in DebugView

This comment has been minimized.

Copy link
@markstory

markstory Jan 12, 2012

Author Member

DebugKit should be updated to check the length of $this->output then I think.

This comment has been minimized.

Copy link
@markstory

markstory Jan 12, 2012

Author Member

Or __isset() should look for the overloaded properties in the same way that __get() does.

This comment has been minimized.

Copy link
@ceeram

ceeram Jan 12, 2012

Member

this shouldnt be fixed in debugkit only, as apps could rely on this features as well

ceeram@2ee0fbc

would solve it

i also changed return for View->action as i think it makes more sense to return null as thats the default value in request->params['action'] as well

This comment has been minimized.

Copy link
@markstory

markstory Jan 13, 2012

Author Member

Won't that change have issues when request->params['action'] is undefined? I haven't tried the code out, only read it.

This comment has been minimized.

Copy link
@ceeram

ceeram Jan 13, 2012

Member

https://github.com/cakephp/cakephp/blob/2.1/lib/Cake/Network/CakeRequest.php#L37 it should have null value by default, thats why i changed the return value as well, its more consistent to return null as well instead of empty string

This comment has been minimized.

Copy link
@markstory

markstory Jan 14, 2012

Author Member

Hmm, right you are. I'll give the change a try and merge it in :)

return isset($this->{$name});
}
/**

0 comments on commit d238d8c

Please sign in to comment.
You can’t perform that action at this time.