[BUG] Search input in "Filter" tab in advanced search drops key presses as of 5.3.5 on Firefox

[hoelzro]

Describe the bug

Note: I currently have only reproduced this on my own wiki, but I thought it would be wise to report it so that others can find it and I can share details/take in suggestions as I try to get to the bottom of this.

After upgrading from TiddlyWiki 5.3.3 to 5.3.5, I noticed that if I use "search by filter" in the advanced search tiddler, typing at my normal speed ignores one of every couple key presses.

I don't see this on Chrome, and I do see this on a fresh Firefox profile.

Expected behavior

No response

To Reproduce

(working on a minimal reproduction)

Screenshots

No response

TiddlyWiki Configuration

Version: 5.3.5
Saving mechanism: Node
Browser: Firefox 129.0.1
OS: Arch Linux
Plugins: Quite a few - more details in the "Additional context" section

Additional context

I used git bisect to identify the commit that introduced the bug (e571239), and then experimented to find the specific change within that commit - if I restore that deleted newline, search-by-filter works fine again, which is confusing, to say the least.

I started up an instance of my wiki with my extra plugins disabled; I managed to figure out that the calendar plugin I am using contributes to the bug. On my wiki, the CSS provided by that plugin seriously impairs the performance of styleRefresh - I actually have an overridden tiddler that "unrolls" that CSS, but even then its presence still significantly increases the styleRefresh timing. The bug shows up whether or not I have the calendar and my override of its CSS tiddler in my wiki, but more keys end up getting dropped when they are present.

The surface-level "fix" of reverting that newline removal works - another "fix" would be to remove the calendar plugin and my overridden CSS for it. But it feels off to me that a slow style refresh causes keyboard inputs to get dropped, rather than just be laggy, so I'm going to try to get to the bottom of this. My wiki isn't exactly small - it's nearing 13K tiddlers in size - and there's plenty of additional customization lurking beneath the surface that I'm sure may contribute to the problem. I'm going to try to create a more minimal reproduction, and will post it here if I manage to do so.

In the meantime, any help or guidance to help me on my quest to figure out what might be going on would be appreciated! Here are some open questions I currently have:

• I noticed that styleRefresh doesn't throttle like mainRefresh does - should it?
• Why is "search by filter" seemingly the only input affected?
• Why only Firefox? Just due to difference in speed?
• Why does the newline affect things? Is the bug just about slowing things down enough to trigger some issue, or is it because it introduces enough difference in the widget tree/DOM to trigger it?

[saqimtiaz]

@hoelzro I am curious if browser addons might be playing a role here, have you tested with all addons disabled?

[hoelzro]

@saqimtiaz That's a good point! I have - I launched a fresh Firefox profile (just using the WebExtension web-ext development tool) and saw the issue there; I also see the issue when I run a reproduction script I wrote using Playwright.

[hoelzro]

Ok, I've managed to spend some time on this this week!

Testing Setup

• I have a more-or-less minimal reproduction (link). It seems that rendering speed affects things, so it might not manifest readily with smaller wikis - rather than creating a "minimal" example with a bunch of tiddlers, I created a <$delay> widget to artificially slow things down.
• Within my reproduction, ExperimentControls can be used to set up how much artificial delay to add to the mainRefresh and styleRefresh phases - it can also be used to change whether a volatile tiddler is used for the state (more on this later). ProblemTiddler is the problematic tiddler that demonstrates the issue. SlowStyle is a stylesheet tiddler that artificially delays styleRefresh.
• I didn't want to affect reproduction of the bug by using variables/references in the example tiddler, so ExperimentControls just uses a template to populate ProblemTiddler and SlowStyle based on the settings.
• I have a Playwright script which takes the URL of a wiki as an argument that I've been using to test the behavior; I was not able to reproduce the bug using Playwright's own keyboard APIs, so as I'm on Linux, I have an xdotool invocation in that script to inject key presses in order to reproduce the issue.

Actual Findings

• I was not able to reproduce this on Chromium, not even with large (~1000ms) delays.
• If I use the devtools console to add keypress event listeners to the <input> element as well as document.body, I notice that the <input> doesn't see the dropped key presses (which makes sense), but document.body does see every one.
• If I use a volatile tiddler to store the keyboard state, the issue goes away, which makes sense, since in that case mainRefresh ends up getting deferred until the typing is complete - however, styleRefresh does not get throttled in this way, and I do not see the issue when using a volatile tiddler along with a high value (eg. 250ms) for the style refresh delay!
• In the reproduction, you'll see there's a <span> element wrapping a <$let> widget:

<$edit-text class="repro-search" tiddler="$:/temp/repro" tag="input" type="search" />
<!-- insert newline here to "fix" -->
<$delay delay="250" noop={{$:/temp/repro}} />
<span>
<$set name="filterLength" value={{{ [{$:/temp/repro}length[]] }}}>
<div>{{$:/temp/repro}}</div>
</$set>
</span>

Removing either of these makes the issue go away - what's interesting here is that if you replace <span> with a block-rendered element (eg. <div>, or even <span style="display: block">), the issue also goes away. Using <div style="display: inline"> causes the issue just like <span> does.

Theories/Next Steps

• The fact that the display CSS property affects things makes me wonder if this is a Firefox bug itself - but I'm not the most knowledgable about this kind of thing!
• The other area I suspect is how TiddlyWiki renders and replaces DOM pages into the page - but I don't know enough about those internals to really weigh in here.
• I mentioned above that the <$let> widget is necessary to reproduce the bug - I'd like to experiment with other types of widgets to see if they do or do not cause the bug.

Any suggestions/insights/guidance around this investigation would be very much appreciated 🙂

[CodaCodr]

FWIW, Windows 10, Firefox 129.0.2...

tw-com, 5.3.5, location: AdvancedSearch filter tab, no issue.
tw-com, 5.3.6-prerelease, location: AdvancedSearch filter tab, no issue.

https://hoelz.ro/files/gh-8551-repro.html location: AdvancedSearch filter tab, no issue.
https://hoelz.ro/files/gh-8551-repro.html location: ProblemTiddler, completely unusable. I tried "volatile" checked and unchecked.
https://hoelz.ro/files/gh-8551-repro.html location: ProblemTiddler having inserted a newline as suggested, no issue.

Not sure what you're doing with the delay widget. I removed it and found no issue (the above tests worked).

[hoelzro]

@CodaCodr Thanks for taking it for a spin! When you say "completely unusable" - did any keypresses show up?

[hoelzro]

I've managed to reproduce this behavior entirely outside of TiddlyWiki: https://jsfiddle.net/5z1ft7ky/

That part leads me to think that this might be a deeper issue with Firefox itself and how it processes events while manipulating the DOM. If that's the case, it's not really TiddlyWiki's responsibility to fix this behavior, but I'm wondering if TiddlyWiki's rendering/DOM manipulation could be changed to work around this issue. I did a simple reproduction using React and noticed that did not run into this issue (so it seems that a fixed version is possible!), but its virtual DOM implementation is obviously significantly different from TiddlyWiki's DOM manipulation code, so that would probably be a lot of work in a pretty core part of TiddlyWiki.

One thing that I did notice that's interesting - when I tried making my standalone reproduction above, I originally laid out the DOM elements using plain HTML, but Firefox actually rearranged the elements. It moved the innermost <div> outside of the <p> element. And when I tried my React version, I got this warning:

06:35:13.055 Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>.
div
span
p
div
App@http://localhost:8001/react-index.html line 24 > injectedScript:15:20 react-dom.development.js:73:32

Considering that my testing has shown different results based on the "block-y-ness" of different elements involved, I'm wondering if Firefox allows certain types of illegal DOM nesting, but has bugs around them.

[Jermolene]

Hi @hoelzro thanks for investigating this. Perhaps this is a good prompt to try to systematically fix all the illegal HTML nesting that occurs in the core. We could extend the test suite to generate some static content builds and run them through an HTML validator.

[CodaCodr]

When you say "completely unusable" - did any keypresses show up?

Things like [tag[HelloThere]] ==> [t[lltr

[CodaCodr]

Perhaps this is a good prompt to try to systematically fix all the illegal HTML nesting that occurs in the core.

'twould be a noble effort, certainly. But considering I have ~50 wikis (my wife has many more) and all I ever use is Firefox (seems like forever), I have never witnessed behaviour like this, locally.

And when you add to that, the ease with which the touted cause is likely to appear...

I can't imagine how it might be prevented AND keep "nice" code layout.

If this were reported at Bugzilla, I would understand totally if the moz devs responded WONTFIX.

[pmario]

If this were reported at Bugzilla, I would understand totally if the moz devs responded WONTFIX.

I think that's not true. With the jsfiddle example imo there is a good chance that they will have a closer look at it.

[Jermolene]

Hi @CodaCodr the motivation for such a change wouldn't just be this Firefox bug. The more basic problem is that the browser behaviour for misnested HTML elements is largely undefined, giving us a vulnerability to future problems. But the primary goal would be to improve interoperability with other HTML tools. For example, if we want to be able to plug TiddlyWiki's HTML output into tools like Pandoc a good starting point is for us to be producing valid HTML.

[pmario]

@Jermolene -- I did create 2 PRs, that directly tackle the problem described here.

• fix typing problem in advanced search filter tab fix typing problem in advanced search filter tab #8577 ... Contains a 3 line change in the existing code that prevents the "faulty behaviour"
• Format code and fix advanced search filter input eats key presses Format code and fix advanced search filter input eats key presses #8578 ... Which fixes the problem and also formats the code to make it human readable.

#8577 should be merged first

[CodaCodr]

Hi @CodaCodr the motivation for such a change

@Jermolene Like I said, noble effort.

@pmario I think you missed "if". Neither of us has a crystal ball.

My concern is the ease with which <p><div>... can appear while trying to maintain "pretty" code layout. I find TiddlyWiki's propensity/tendency to apply <p>..</p> hard to dodge. After all, I mostly want it to do what it already does...

[pmario]

There is a draft PR: WIP Improve redundant p tags #7061 where I think I have identified the problem.

When the PR was opened and it would have been merged, the TW UI would have had at least 50 severe formatting problems.

I did a new test yesterday, with the current master branch. We are down to 3 or 4 problems. Mainly with the editor toolbar buttons. So I think we are on a good way to finally eliminate redundant html P-tags from the UI.

[CodaCodr]

That's quite some work and diligence you put into #7601, @pmario. Bravo.

[hoelzro]

Hi everyone,

I've been digging into this on and off for the past few weeks, and I've made some interesting discoveries!

• I believe I have found an already-reported instance of this bug; see https://bugzilla.mozilla.org/show_bug.cgi?id=578663 (sounds very similar!)
• The bad news is that I'm able to reproduce this without illegal HTML - as far as I can tell, the bug manifests when code adds a block node (eg. a <div>) under an inline node (eg. a <span>).
• The good news is that my minimal reproduction with TiddlyWiki can be changed to avoid the bug by removing the <$set> widget - so I'm wondering if SetWidget can be changed to re-use existing DOM nodes more to avoid triggering this bug, but how/when DOM elements end up getting re-used is an area of the TiddlyWiki codebase I'm not super familiar with.

[Jermolene]

Thanks @hoelzro that is interesting. The <$set> widget doesn't actually generate any DOM elements, so I suspect that the problem might be that the <$set> widget in question is formatted so that the content is parsed in block mode (for example, this could be done with a double line break after the opening tag of the <$set> widget.

[hoelzro]

Sorry, I think I misspoke - from my understanding, <$set> itself doesn't generate any DOM elements, but it can result in completely new DOM elements getting generated for its children, right? For example:

<$set name="filterLength" value={{{ [{$:/temp/repro}length[]] }}}>
<div>{{$:/temp/repro}}</div>
</$set>

If $:/temp/repro changes, the SetWidget will end up creating a new HTMLElement for that <div> - but if we remove the surrounding <$set>…</$set> (or change it so that value="0" or something), the same HTMLElement will be used for the <div>.

Is my understanding correct here?

[pmario]

@hoelzro -- I think what you describe is what I've done with the "temporary" fix at: https://github.com/TiddlyWiki/TiddlyWiki5/pull/8577/files

Removing the DIV child from the set-widget and moving it "outside" the set into the "reveal-widget". It seems to work, but as I wrote -- I do not really know why.

So thanks for your digging.

---

I think with the jsfiddle from your comment #8551 (comment) you will be able to describe the bug in a way that FF developers actually listen. So you should create an issue at bugzilla where you link to jsfiddle.

[hoelzro]

@pmario Yes, thanks for doing that! And I think you're right regarding submitting my jsfiddle - I need to get around to that 😅

I'm kind of wondering if we address this bug at the widget refresh/DOM manipulation level - for example, can we get widgets like SetWidget to call refreshSelf() less often, or perhaps even change refreshSelf() and its descendants in the call graph to re-use existing DOM nodes more? That sounds like a pretty big change, but it might be worth it - not just to address this bug, but for a potential speed-up!