Toggl-button: Performance regression for Todoist integration

[mgsloan]

• OS version: Ubuntu 18.04.1 LTS
• Browser version: google-chrome Version 71.0.3578.98 (Official Build) (64-bit)
• Extension version: 1.17.1

Bug description

When selecting a parent project in Todoist that has nested sub-projects and many tasks, Todoist becomes unresponsive. Via chrome developer tools I tracked this down to being due to toggl-button. Disabling the extension resolves the problem.

I believe that this is a recent regression related to changes in Todoist version 935. https://get.todoist.help/hc/en-us/articles/115005442125-Todoist-for-Web-Changelog

🐛 You may notice that Todoist runs more smoothly across the app thanks to some performance optimizations.

I'm pretty familiar with Todoist's behavior (I wrote a fairly large extension for it, todoist-shortcuts). I believe that what's changed here is that now it creates the dom nodes for the tasks sequentially, possibly in batch, instead of creating them all at once. Since toggl-button uses a mutation observer, it's seeing many more mutations than it used to, and re-rendering all of the toggl buttons. So I'm guessing this results in O(n^2) runtime when the tasks get rendered, because toggl-button seems to modify every single task on every single dom mutation.

Expected behavior

I expected Todoist + toggl-button to not hang when viewing many tasks.

Steps to reproduce

1. Install toggl-button

2. Visit a todoist project with many tasks, particularly with nested projects

3. Observe that the browser goes out to lunch, profiling reveals it's very busy.

[tcrammond]

Thank you for the detailed report 🙌 This is being looked in to.

[mgsloan]

@tcrammond Great, thanks!

I have an idea for how to implement this efficiently. I noticed that the toggl-button is only displayed when a task is hovered. So, one option here might be to just register a mouseover event handler on the #editor div. I noticed that event propagates up to that div. I tried this by selecting it in the dev tools and running $0.addEventListener("mouseover", function(e) { console.log("it works", e); }).

The mouseover handler would then take the target of the event and walk up the dom until it finds a node that has class task. It would then add the toggl button to that task. There are multiple ways to handle the prior displayed button (if any), probably best to just remove the old dom nodes for those if they still exist (they might not!).

This approach should entirely avoid any processing that scales with the number of displayed tasks, and wouldn't require a mutation observer.

[Airgunster]

Can someone please update current status of this issue? Looks like the fix is not included into the latest version (v. 1.22.1 (as of 2019-02-27 17-00 GMT)).

[shantanuraj]

@Airgunster the fix is present in version 1.22.1

It landed via #1321 and #1282

It seems the auto-close comment was not added to the PRs

[Airgunster]

Thanks a lot for everyone who was working on this, it now works like a charm (very smooth and without any glitches)!