-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Crash on empty transclusion syntax inside button widget #8144
Comments
I can confirm the problem. With FF on Widows, I can still close the preview-area. So the wiki stays responsive. FireFox on ubuntu 22.04 has a really hard time. It's close to impossible to get the wiki "unbricked" The current MAX_WIDGET_TREE_DEPTH inside a button-widget can cause data loss. I do have an idea, which would allow us to create a "per-widget" max depth limit. Currently the code is simple. Do you think it's worth to create a PR? |
I've prepared a PR that takes a different approach and resolves this at the parsing level, I'll commit the changes shortly. |
Thanks @bluepenguindeveloper @pmario I've prepared a fix over at #8145, please test it if you can. |
@Jermolene My idea can be seen in the screenshot. Since "button inside button inside a button ... " does not make much sense, we know that we should stop it early. In the screenshot it stops at ancestorCount() 334, which should not cause too many problems. But I would need to test this one with FF on ubuntu again. The idea would need some refinement. Since I think "button in button" should only be allowed eg: 10 times for any weird usecase. I think Just some thoughts. |
Hi @pmario in my tests, your example of |
I did not hardcode the constant. I used There is a Talk thread Repeat expression (n) times? where Scott Sayet creates an interesting recursive function collatz(x) where our hadcoded limit 1000 actually creates a "false positive" check. So I did suggest to create a possibility to actually "widen" the limit. I think it should be possible to change the limit very specifically. My suggestion was a |
Here is the full code I used in the button.
Where the widget check uses a "variable" and looks like this:
|
This screenshot is not possible at tiddlywiki.com atm. But it shows the possibility to dynamically raise the max widget tree depth and still use buttons within the higher limits. The button-widget uses a dynamic maximum of +20 to the existing maximum. The This allows us to still use buttons inside the "test-collatz" example. |
Describe the bug
Wikitext such as
<$button>{{}}</$button>
(or unclosed button<$button>{{}}
) results in an immediate crash.Found and tested with single file wiki on Windows 10 with TiddlyDesktop and the Brave browser (both Chromium-based)
Expected behavior
Recursive transclusion error (without crashing TiddlyWiki)
To Reproduce
<$button>{{}}</$button>
or<$button>{{}}
into the text fieldSteps 2 and 3 can be done in reverse order, in which case, the crash will occur as soon as the second
}
is typed if typing manually.Screenshots
No response
TiddlyWiki Configuration
TW Version [e.g. v5.3.3]
Saving mechanism: TiddlyDesktop, or none (occurs if opening single file wiki in the browser (Brave), and can be done in the TiddlyWiki documentation simply viewed in the browser); seems to be independent of saving mechanism
OS: Windows 10
Browser: Brave (Version 1.64.116; Chromium: 123.0.6312.105)
Additional context
No response
The text was updated successfully, but these errors were encountered: