help me fix it, please
f7o7
tiddler shifts up, but all the content of text fields remains at the
previous position and now overlaps with the tiddler below.
I am using Firefox 2.
f7o7
the mouse to an above tiddler, the text jumps up to the proper
position.
Daniel Baird
and fiddle around there, do you get the same thing?
;D
--
Daniel Baird
http://tiddlyspot.com (free, effortless TiddlyWiki hosting)
http://danielbaird.com (TiddlyW;nks! :: Whiteboard Koala :: Blog ::
Things That Suck)
Bob McElrath
>
> Does it happen in every TiddlyWiki? Like, if you go to tiddlywiki.com
> and fiddle around there, do you get the same thing?
I see this often. I think it's a firefox bug.
--
Cheers,
Bob McElrath [Univ. of California at Davis, Department of Physics]
"The reasonable man adapts himself to the world; the unreasonable one
persists in trying to adapt the world to himself. Therefore all progress
depends on the unreasonable man."
-- George Bernard Shaw, Maxims for Revolutionists
f7o7
EnableAnimations. If it's turned off, then this happens. So what's the
scoop?
f7o7
them faster? Maybe do it in a couple of steps instead of 10 or
whatever it's set to? (I am already using Eric's CoreTweaks and turned
the fading off, but it's still slow)
Jeremy Ruston
took it to be a Firefox bug.
I'd much appreciate any attention anyone can give to figuring out a
way to reproduce this problem reliably so that we can report it to
Mozilla. It may also be possible to fashion a workaround.
Cheers
Jeremy
--
Jeremy Ruston
mailto:jer...@osmosoft.com
http://www.tiddlywiki.com
f7o7
1. Go to http://www.tiddlywiki.com
2,3. Open up options, uncheck EnableAnimations
4. Click "view" on GettingStarted
5. Close Raves
Observe
FND
Thanks for the description.
Confirmed for Firefox 1.5.0.9 (FWIW).
-- F.
Edgar Javison
Edgar Javison
We are each of us angels with only one wing, and we can only fly by embracing one another. ~ Luciano de Crescenzo
If you've got time, do drop by my weblog at http://elj.tiddlyspot.com/
Jeremy Ruston
http://trac.tiddlywiki.org/tiddlywiki/ticket/301
Cheers
Jeremy
On 2/17/07, f7o7 <ewi...@gmail.com> wrote:
>
Udo Borkowski
in Story.prototype.closeTiddler replace
else
tiddlerElem.parentNode.removeChild(tiddlerElem);
by
else
{
//# see Ticket #301
tiddlerElem.style.overflow = "hidden";
setTimeout(function(){tiddlerElem.parentNode.removeChild(tiddlerElem)},0);
}
Jeremy: I updated the ticket accordingly. You may want to apply this fix for 2.2.
Udo
----------
Udo Borkowski
http://www.abego-software.de
Eric Shulman
> in Story.prototype.closeTiddler replace
> by
> tiddlerElem.style.overflow = "hidden";
> setTimeout(function(){tiddlerElem.parentNode.removeChild(tiddlerElem)},0);
EEK!
While this may seem like a simple tweak, this 'fix' means that
removing a displayed tiddler is now performed asynchronously, and any
code that was calling on closeTiddler() can no longer safely assume
that the corresponding tiddler display elements are actually gone from
the DOM tree by the time the function returns.
This problem shows up in http://www.TiddlyTools.com/#BreadcrumbsPlugin,
where pressing the "Home" link in the breadcrumbs list will close all
open tiddlers, but fails to re-display the DefaultTiddlers. Here's
what's happening:
BreadCrumbs defines "restartHome()", which includes these two lines:
story.closeAllTiddlers();
restart();
closeAllTiddlers() iterates over the story elements and invokes
closeTiddler() on each. Then, restart() checks for story.isEmpty()
before re-displaying the DefaultTiddlers. However, because the fix
for #301 defers the actual element removal, story.isEmpty() still
finds tiddler elements and reports false, even though those tiddler
elements WILL be removed as soon as the timeouts set by closeTiddler()
get triggered.
Although the problem MIGHT be isolated to this one use-case, it will
also have a similar impact on any other use-cases that relied on
closeTiddler() to have an invariant return condition that assures no
tiddler story elements remain after the call is completed.
Given TiddlyWiki's ever-increasing "installed base", it is even more
important to carefully evaluate (and test) the effect of every core
change, no matter how minor it seems, before it is added into the
official codebase where it might create more problems than it fixes!
-e
Eric Shulman
TiddlyTools / ELS Design Studios
Udo Borkowski
While this may seem like a simple tweak, this 'fix' means that
removing a displayed tiddler is now performed asynchronously, and any
code that was calling on closeTiddler() can no longer safely assume
that the corresponding tiddler display elements are actually gone from
the DOM tree by the time the function returns.
Therefore I considered this a safe change.
Nevertheless I wonder why the problem you are reporting (regarding the Breadcrumbs plugin) doesn't occur in the current TW (with animation enabled).
... it is even more
important to carefully evaluate (and test) the effect of every core
change, no matter how minor it seems, before it is added into the
official codebase where it might create more problems than it fixes!
I think we all agree on that. I tested the stuff on the systems I have available (FireFox 1.5/IE 6, Windows), using a standard TW installation. Obviously I am not able to test the stuff with every plugin, but from what I learned from the past plugin-related-problems will quickly be reported by the community. Depending on the cause of the problem it is either fixed in the core (as often as possible), or by a new version of the plugin.
In the given case I assume the fix should be done in the Breadcrumb plugin, since it relies on an assumption that was never true (in general). My suggested fix is:
in BreadCrumbs define "restartHome()" as:
function restartHome(){
story.closeAllTiddlers();
setTimeout(function() {
restart();
config.breadCrumbs = [];
var crumbArea = document.getElementById("breadCrumbs");
if (crumbArea) // ELS: added check to make sure crumbArea exists
crumbArea.style.display = "none"; // ELS changed from: crumbArea.style.visibility = "hidden";
}, 0);
Udo Borkowski
If it is really necessary to run code "after" the tiddler to close is actually gone I assume one either needs to do some polling or extended the closeTiddler function. E.g. pass in a callback function to be called when the tiddler is really closed:
Story.prototype.closeTiddler = function(title,animate,slowly,callback)
Because the way stuff is currently implemented this would also require a change to the Slider class, since it must call the callback "on stop". Might also be interesting for other applications.
Eric Shulman
> Breadcrumbs plugin) doesn't occur in the current TW (with animation
> enabled).
I think it is really important to understand why this breadcrumbs
"home" problem *doesn't* exist using the current 2.1.3 build with
animation enabled. Perhaps it is means that your assertion that
closeTiddler() is already asynchronously handled is somehow incorrect
(though your analysis does seem to make sense to me as well)
> > important to carefully evaluate (and test) the effect of every core
> > ...
> I think we all agree on that. I tested the stuff on the systems I have
> available (FireFox 1.5/IE 6, Windows), using a standard TW installation.
I didn't mean to suggest that you were lax in your testing... sorry if
it came out that way... I know that it is hard to cover all the
possible platforms and use-cases, especially in an open source project
with so many active contributors.
> In the given case I assume the fix should be done in the Breadcrumb plugin,
> function restartHome(){
> story.closeAllTiddlers();
> setTimeout(function() {...}, 0);
> }
Is a zero timeout value somehow handled differently by the browser so
that it can be queued up to trigger AFTER all timed triggers have
fired? Otherwise, what would happen if it takes a while to animate
the closing of a tiddler? If the breadcrumb handling isn't somehow
delayed until after the animation has finished, the tiddler DOM
element might still exist when the breadcrumb timeout is triggered,
and restart() should still fail.
hmm....
Eric Shulman
> actually gone I assume one either needs to do some polling or extended the
Perhaps this might work in the BreadcrumbsPlugin restartHome():
...
story.closeAllTiddlers();
while (!story.isEmpty()) continue; // poll for animation to finish
restart();
...
what do you think?
-e
Eric Shulman
> "home" problem *doesn't* exist using the current 2.1.3 build with
> animation enabled. Perhaps it is means that your assertion that
> closeTiddler() is already asynchronously handled is somehow incorrect
> (though your analysis does seem to make sense to me as well)
Ah hah! I think I've sussed it!
Breadcrumbs' restartHome() function calls on story.closeAllTiddlers(),
which then invokes story.closeTiddler(title) for each displayed
tiddler. Note that there is an optional second parameter for
closeTiddler(), a boolean flag that has to be TRUE for animation to be
used.
Howvever, story.closeTiddler() is being called with only one param
(the tiddler title), so this flag defaults to FALSE and
story.closeAllTiddlers() *never* used animation. Thus, processing of
story.closeAllTiddlers() always occured synchronously, ensuring that
all tiddlers were actually gone from the DOM tree by the time the
function returns.
This means that the 301 change DOES alter the semantics of the core
behavior for story.closeAllTiddlers() by making it operate
asynchronously, even though animation is NOT used (or desired) in this
case.
I think the best way to fix this would be to re-write
closeAllTiddlers() so that it bypasses the underlying closeTiddler()
call so it can directly implement a synchronous close of all tiddlers,
regardless of timeouts that are used for animation or the #301 fix...
Story.prototype.closeAllTiddlers = function(exclude)
{
clearMessage();
this.forEachTiddler(function(title,element) {
if((title != exclude) && element.getAttribute("dirty") != "true") {
this.scrubTiddler(element);
element.parentNode.removeChild(element);
}
});
window.scrollTo(0,0);
Eric Shulman
Actually, this gets STUCK in any infinite loop if there are any open
tiddler *editors*, since closeTiddler() will NOT close those tiddlers
(to protect against throwing away any changes those editor textareas
may contain).
Infinite loops are bad. :-(
-e
Eric Shulman
> closeAllTiddlers() so that it bypasses the underlying closeTiddler()
> call so it can directly implement a synchronous close of all tiddlers,
> regardless of timeouts that are used for animation or the #301 fix...
As a test, I've added the #301 changes, as well as redefining
story.closeAllTiddlers():
http://www.TiddlyTools.com/#CoreTweaks
I'm happy to report that, in combination, these two changes will fix
#301 while still preserving the existing synchronous behavior needed
by the BreadcrumbsPlugin "home" button. I think we should make the
closeAllTiddlers() change part of the #301 fix, so that it won't break
the existing installed base of BreadcrumbsPlugin users (or force them
to have to patch/update the plugin) when they upgrade their core to
TW2.2. Do you concur?
-e
Udo Borkowski
Note that there is an optional second parameter for
closeTiddler(), a boolean flag that has to be TRUE for animation to be
used.
AHHH! That's it.
I think the best way to fix this would be to re-write
closeAllTiddlers() so that it bypasses the underlying closeTiddler()
call so it can directly implement a synchronous close of all tiddlers,...
That's a one way to solve the issue.
Alternatively we may add a parameter "allowAsync" both to closeTiddlers and closeAllTiddlers. E.g.
Story.prototype.closeTiddler = function(title,animate,slowly,allowAsync)
and
Story.prototype.closeAllTiddlers = function(exclude,allowAsync)
When allowAsync is false (or missing) the old behaviour is performed, possibly leading to the "Firefox artefacts" #301 talks about. When set to true the code I suggested in #301 is used.
This way every old code that calls closeTiddler/closeAllTiddlers will observe the exact same behaviour as before. New code (or the new core code) that is aware of the allowAsync can decide if it is safe to close the tiddler(s) asynchrously (and thus avoid problems with FireFox rendering). E.g. in
* config.macros.closeAll.onClick, and
* config.commands.closeOthers.handler
I would allowAsync for closeAllTiddlers, in
* Story.prototype.search
I would not allowAsync.
(Similar one will check for calls to closeTiddler. E.g. config.commands.closeTiddler.handler would pass true with allowAsync)
This approach has the advantage that also the closeAllTiddlers function can run without Firefox artefacts, e.g. in case the "closeOthers" command or the "closeAll" macro are used by the user. It also avoids duplication of code in closeAllTiddlers (just needs to pass through the allowAsync parameter).
What do you think?
Eric Shulman
> closeAllTiddlers. E.g.
> When allowAsync is false (or missing) the old behaviour is performed,
> This way every old code that calls closeTiddler/closeAllTiddlers will
> observe the exact same behaviour as before. New code (or the new core code)
> that is aware of the allowAsync can decide if it is safe to close the
> tiddler(s) asynchrously (and thus avoid problems with FireFox rendering).
> This approach has the advantage that also the closeAllTiddlers function can
> run without Firefox artefacts, e.g. in case the "closeOthers" command or the
> "closeAll" macro are used by the user. It also avoids duplication of code in
> closeAllTiddlers (just needs to pass through the allowAsync parameter).
> What do you think?
I think that is a much nicer way to handle this... so much so, that
I've just re-written the TiddlyTools CoreTweaks to do as you have
described. It seems to be working well. If you like the code I've
written, feel free to submit it as a core patch for the #301 bug...
-e
Udo Borkowski
If you like the code I've
written, feel free to submit it as a core patch for the #301 bug...
Many thanks, Eric, I will do so.
Just a question: at the end of your closeAllTiddlers it says
window.scrollTo(0,0);
rather than
window.scrollTo(0,ensureVisible(this.container));
Is this an intended change?
Eric Shulman
> window.scrollTo(0,0);
> rather than
> window.scrollTo(0,ensureVisible(this.container));
>
> Is this an intended change?
This is an intended NON-change copied as-is from the current TW2.1.3
core. The purpose of scrolling to (0,0) is to ensure that, after
closing ALL tiddlers, the very top of the page, including the header
area, will be visible.
-e
Udo Borkowski
This is an intended NON-change copied as-is from the current TW2.1.3 core
Thanks,
Udo Borkowski
Many thanks again to Eric for his input,
f7o7
Follow these steps:
1. Go to http://tiddlytools.com/#MainMenu%20Welcome
2. Open up options, display
3. Uncheck animation
4. Click "view" on Welcome
5. Click "fold" and "unfold" repeatedly on MainMenu tiddler.
Watch the text trying to catch up with textareas!