enumerated Property:Page only works for first allows_value
Closed, ResolvedPublic


Author: info

Yaron Koren in http://www.mail-archive.com/semediawiki-devel@lists.sourceforge.net/msg00319.html pointed out that there's a bug in the handling of properties of type "Page" that are enumerations - fields that have this property get a "not on the list of possible values" error on the screen if they have a value other than the very first one on the list.

I reproduced, see URL.

Yaron comments:

I traced this problem, and found the issue: a variable is created, in "SMW_DataValue.php", to loop through the list of allowed values, checking each one against the current value. In the case of a wiki page, however, that variable keeps a cached version of its title, that is set the very first time and then is never reset. There are are various ways to solve this problem, but the easiest might be to change the function parseUserValue() in the file "/includes/SMW_DV_WikiPage.php". You can add a new line after line 26, so instead of

$this->m_value = $value;

...it now reads

$this->m_value = $value;
$this->m_title = null;

This will prevent the old value of "m_title" from getting read and used in place of the actual value.

skierpage comments:
This fix works great for me. However, there are other member variables that parseUserValue() may leave untouched, though mainly in an error case where maybe it doesn't matter. Which functions should reset a datavalue, setUserValue() or parseUserValue(), or both?

Version: unspecified
Severity: major
URL: http://ontoworld.org/wiki/Test_allows_value_wikipage

bzimport set Reference to bz11820.
bzimport created this task.Via LegacyOct 30 2007, 10:15 PM
bzimport added a comment.Via ConduitOct 30 2007, 10:51 PM

info wrote:

I checked in Yaron's fix to SMW_DV_WikiPage.php, revision 27069.

bzimport added a comment.Via ConduitJan 5 2008, 1:13 PM

info wrote:

James McMurtrie found this issue broke again, see http://www.nabble.com/%27Allows-value%27-broken-in-1.0--to14632232.html

I confirmed with the same URL, so reopening.

The issue is exactly the same as before. The datavalue variable that loops through the allowed values doesn't reset and/or update all its state on setUserValue(), so only the "first" allows value (as with any special property, the values return from the database in random order) matches.

I don't know the general way to fix this. To repeat:

'''Which functions should reset a datavalue, setUserValue() or parseUserValue(), or both?'''

bzimport added a comment.Via ConduitJan 5 2008, 1:45 PM

info wrote:

*IF* parseUserValue() is the place to reset, then *A* fix is in SMW_DV_WikiPage.php's parseUserValue(), before it calls getTitle(), set $this->m_dbkeyform = NULL; as well. Otherwise getTitle() creates a title from the old cached value of m_dbkeyform().

But, see my original "skierpage comments:"

Yaron_Koren added a comment.Via ConduitJan 17 2008, 4:39 PM

I definitely think S Page's suggested solution, of adding the line "$this->m_dbkeyform = NULL;", should be applied to the code for now, regardless of what the "ideal" solution is.

mkroetzsch added a comment.Via ConduitApr 17 2008, 7:09 PM

Oops, that bug was open for too long, given that it was a one-liner. Fixed now in SVN.

I think the clean way would be to have some function "clearValue" for all datatypes that is then used in set*Value(). Maybe I will do it like that soon.

bzimport added a comment.Via ConduitJul 10 2008, 10:52 AM

cwdillon wrote:

Our wiki is experiencing the symptoms of this bug again. It was failing in SMW 1.0.1 and we replicated it on our test server after upgrading to SMW 1.2. We can try the patch Yaron provided, above to solve our site problems, but thought you'd want to know that the bug seems to still be there.


bzimport added a comment.Via ConduitJul 10 2008, 12:31 PM

cwdillon wrote:

Aargh! Turns out that the programmer and I were working off of two different test sites; no matter what he tried on the "development" site, the problem persisted on the "test" site. Nothing to see here. Pay no attention to the new guy.

Add Comment