Skip to content

Commit

Permalink
Fixed problem with widgets variable access
Browse files Browse the repository at this point in the history
Previously, widgets were reading variables from themselves or their
cascaded ancestors. That means that if a widget sets a variable and
then reads the same variable, it will get the same variable back. That
sounds reasonable, until you consider a widget that wants to modify a
variable - eg the tiddler macro. For example:

```
<$tiddler tiddler={{!!report}}>
<$transclude mode="block" />
</$tiddler>
```

Here we first evaluate the `{{!!report}}` reference, which involves
reading the currentTiddler variable, looking up the tiddler, and
retrieving it’s `report` field. The next the tiddler widget is
refreshed, it will use the newly set currentTiddler as the basis for
resolving the `{{!!reference}}` reference.

The fix is to get variables from ancestors, but continue to set them on
ourselves.
  • Loading branch information
Jermolene committed May 8, 2014
1 parent 6ab68e0 commit e60fc9f
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions core/modules/widgets/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,19 @@ defaultValue: default value if the variable is not defined
*/
Widget.prototype.getVariable = function(name,options) {
options = options || {};
var actualParams = options.params || [];
// If the variable doesn't exist then look for a macro module
if(!(name in this.variables)) {
return this.evaluateMacroModule(name,actualParams,options.defaultValue);
var actualParams = options.params || [],
parentWidget = this.parentWidget;
// Check for the variable defined in the parent widget (or an ancestor in the prototype chain)
if(parentWidget && name in parentWidget.variables) {
var variable = parentWidget.variables[name],
value = variable.value || options.defaultValue;
// Substitute any parameters specified in the definition
value = this.substituteVariableParameters(value,variable.params,actualParams);
value = this.substituteVariableReferences(value);
return value;
}
var variable = this.variables[name],
value = variable.value || "";
// Substitute any parameters specified in the definition
value = this.substituteVariableParameters(value,variable.params,actualParams);
value = this.substituteVariableReferences(value);
return value;
// If the variable doesn't exist in the parent widget then look for a macro module
return this.evaluateMacroModule(name,actualParams,options.defaultValue);
};

Widget.prototype.substituteVariableParameters = function(text,formalParams,actualParams) {
Expand Down

4 comments on commit e60fc9f

@sukima
Copy link
Contributor

@sukima sukima commented on e60fc9f May 10, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did a git bisect and this commit breaks the build. Crashes with:

Executing task: render platforms: browser after: story (index):5719
Uncaught TypeError: Cannot read property 'replace' of undefined (index):4082
Uncaught TypeError: Cannot read property 'replace' of undefined $:/core/modules/widgets/widget.js:140

Trace log: https://gist.github.com/sukima/17275a038e502fdc91d7

@Jermolene
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sukima - what command/script is giving you that error?

@sukima
Copy link
Contributor

@sukima sukima commented on e60fc9f May 13, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jermolene:

$ git pull
$ git checkout e60fc9f
$ npm link
$ cd /tmp
$ mkdir foobar
$ cd foobar
$ tiddlywiki --init server
$ tiddlywiki --server

The above error is seen in the Chrome Dev Tools console after opening http://localhost:8080/

Chrome Version 34.0.1847.131 on Mac OS X 10.9.3

@Jermolene
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sukima fix coming up momentarily.

Please sign in to comment.