Loses focus from text field repeatedly

[Mach3Maelstrom]

This happens occasionally after I open todotxt.net and after I create a task for the first time in that session.

Every time I try to enter a new task after that, the focus is taken off the text field and onto an item on my todo.txt. When I try to refocus it, it ends up being a tiny tug-of-war where I'm clicking in the text field and focus is reattributed to an item on my list. This happens whether I click in the text field or use the shortcut N.

Mashing Escape occasionally rectifies this issue, allowing me to type new tasks. Sometimes that doesn't work and I have to restart todotxt.net so I can enter a new task.

[benrhughes]

Hi, Are you using the (just released) v3.0? We had a similar issue in previous versions, which should have been fixed in 3.0, but you have me worried that I may have made things worse with my fix.

If you're not using it, there's an option (in File -> Options) called 'Move focus to task list after creating a new task'. Obviously getting into a weird event-war loop isn't good, but try disabling that first and see if it works for you.

Cheers,

Ben

[Mach3Maelstrom]

Hi Ben,

I had just installed the newest version and tested it out before leaving my feedback. I unchecked that option and the event-war loop hasn't occurred for the past two days.

I'll follow up if I have any other problems.

P.S.: MAN you're fast with getting back to issues. Thanks for all your hard work. You're the man!

Thanks, Frank

On Tue, Oct 28, 2014 at 6:25 PM, Ben Hughes notifications@github.com wrote:

Hi, Are you using the (just released) v3.0? We had a similar issue in previous versions, which should have been fixed in 3.0, but you have me worried that I may have made things worse with my fix.

If you're not using it, there's an option (in File -> Options) called 'Move focus to task list after creating a new task'. Obviously getting into a weird event-war loop isn't good, but try disabling that first and see if it works for you.

Cheers,

Ben

— Reply to this email directly or view it on GitHub https://github.com/benrhughes/todotxt.net/issues/239#issuecomment-60842659 .

[Piscean]

I'm seeing something very similar. In my case it happens regardless of the 'Move focus to task list' setting.

To repro:

1. Create a todo list with 6 or 8 items.
2. Highlight one and press 'x' to delete it
3. As soon as it clears from the list, press the down arrow a few times to step down to the 4th or 5th item in the list. --> What I see is the focus will step down to the 2nd, maybe 3rd item in the list, then jump back to 1. If you keep pressing the down arrow once or twice per second, the focus continues to step down 2 or 3 items before jumping back to the top.

[benrhughes]

Interesting, thanks. We did change how the UI events were handled recently; maybe it introduced a slight delay that's messing with things. On Nov 1, 2014 12:00 PM, "Piscean" notifications@github.com wrote:

I'm seeing something very similar. In my case it happens regardless of the 'Move focus to task list' setting.

To repro:

1. Create a todo list with 6 or 8 items.
2. Highlight one and press 'x' to delete it
3. As soon as it clears from the list, press the down arrow a few times to step down to the 4th or 5th item in the list. --> What I see is the focus will step down to the 2nd, maybe 3rd item in the list, then jump back to 1. If you keep pressing the down arrow once or twice per second, the focus continues to step down 2 or 3 items before jumping back to the top.

— Reply to this email directly or view it on GitHub https://github.com/benrhughes/todotxt.net/issues/239#issuecomment-61350737 .

[mjdescy]

This bug predates the changes to how the UI events were routed. I have tried to identify the source and I haven't seen anything in the code that would reset focus to the table.

[Piscean]

This problem showed up between 2.3.1.0 and 3.0.0.0. I loaded 2.3.1.0 and it's fine.

I notice the new version is selecting the first item in the list when focus returns, whereas 2.3.1.0 didn't. What I mean is, in 2.3.1.0, if I 'x' task number 3, it archives, the bottom part of the list moves up, and task number 3 is still selected.

In 3.0.0.0, if I 'x' task number 3, it archives, and task 1 is highlighted. The place we're setting the focus to the first item in list feels like a good place to start looking. It's trying to highlight item 1, and when I press the down arrow key, it fights to highlight item 1 again.

I'm sorry I don't have time to poke around with this, we're heading to the airport very shortly.

[mjdescy]

The reason the I/O is redundant is because I tried to change as little as possible of the core functionality (the plumbing) when I added multiple selection. That is not to say that is should be that way--certainly not if it causing problems.

Todotxt.net's philosophy is to prioritize safety and integrity of the todo.txt file above all else (such as speed or low I/O). I didn't want to introduce bugs related to buffering and flushing the file, so it just iterates saves and reloads.

Thanks for your work so far, and I hope you'll continue to help us fix this bug.

On Nov 1, 2014, at 5:19 PM, Piscean notifications@github.com wrote:

I poked around a little bit more. The debug logging was very insightful. Here is the interesting part of the log based on my simple repro as outlined 4 comments above.

[11/1/2014 2:11:57 PM] Task 'x 2014-11-01 due:2014-10-31 test2' deleted

[11/1/2014 2:11:57 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Sorting 6 tasks by None.

[11/1/2014 2:11:58 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Sorting 6 tasks by None.

[11/1/2014 2:11:59 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:59 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:59 PM] Sorting 6 tasks by None.

[11/1/2014 2:12:00 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:00 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:00 PM] Sorting 6 tasks by None.

[11/1/2014 2:12:01 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:01 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:01 PM] Sorting 6 tasks by None.

For every deletion, we are reloading the file and sorting it 6 times. In my brief testing, we are probably resetting focus to the top line of the list about 6 times.

I haven't looked at the load/sort code, but do we need to loop through it 6 times? And if we do, we can probably fix this issue by highlighting the list at the end, rather than each time through.

— Reply to this email directly or view it on GitHub.

{"@context":"http://schema.org","@type":"EmailMessage","description":"View this Issue on GitHub","action":{"@type":"ViewAction","url":"https://github.com/benrhughes/todotxt.net/issues/239#issuecomment-61383842","name":"View Issue"}}

[Piscean]

Yeah, it makes sense. I'd much rather have my data be safe.

I was able to spend a bit more time with it before we left for the airport, but I didn't find anything significant. I'll be back in a week and if you haven't figured it out by then, I'll spend some more time on it.

Eric

On Sat, Nov 1, 2014 at 14:52 PM, Michael Descy notifications@github.com> wrote:

The reason the I/O is redundant is because I tried to change as little as possible of the core functionality (the plumbing) when I added multiple selection. That is not to say that is should be that way--certainly not if it causing problems.

Todotxt.net's philosophy is to prioritize safety and integrity of the todo.txt file above all else (such as speed or low I/O). I didn't want to introduce bugs related to buffering and flushing the file, so it just iterates saves and reloads.

Thanks for your work so far, and I hope you'll continue to help us fix this bug.

On Nov 1, 2014, at 5:19 PM, Piscean wrote:

I poked around a little bit more. The debug logging was very insightful. Here is the interesting part of the log based on my simple repro as outlined 4 comments above.

[11/1/2014 2:11:57 PM] Task 'x 2014-11-01 due:2014-10-31 test2' deleted

[11/1/2014 2:11:57 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Sorting 6 tasks by None.

[11/1/2014 2:11:58 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:58 PM] Sorting 6 tasks by None.

[11/1/2014 2:11:59 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:59 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:11:59 PM] Sorting 6 tasks by None.

[11/1/2014 2:12:00 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:00 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:00 PM] Sorting 6 tasks by None.

[11/1/2014 2:12:01 PM] Loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:01 PM] Finished loading tasks from C:\temp\todotxt.net-dev 10.31.2014\Testtodo.txt.

[11/1/2014 2:12:01 PM] Sorting 6 tasks by None.

For every deletion, we are reloading the file and sorting it 6 times. In my brief testing, we are probably resetting focus to the top line of the list about 6 times.

I haven't looked at the load/sort code, but do we need to loop through it 6 times? And if we do, we can probably fix this issue by highlighting the list at the end, rather than each time through.

— Reply to this email directly or view it on GitHub.

{"@context":"http://schema.org [http://schema.org]","@type":"EmailMessage","description":"View this Issue on GitHub","action":{"@type":"ViewAction","url":"https://github.com/benrhughes/todotxt.net/issues/239#issuecomment-61383842","name":"View Issue"}} — Reply to this email directly or view it on GitHub [https://github.com/benrhughes/todotxt.net/issues/239#issuecomment-61385121].

Sent from my phone

[Piscean]

I think I see what's causing this, but I'm not familiar enough with the code to know exactly what's going on. Hopefully you guys can explain this to me.

In MainWindowViewModel.cs, look at line 67. (_changefile.OnFileChanged += () => _window.Dispatcher.BeginInvoke(new Action(ReloadFile));)

If you comment this line out, it fixes the first item being selected over and over. It also removes the 5 or 6 repeated calls to open the file and sort it.

It feels like this line is creating a race condition. If I understand the code correctly, it is watching for a file change, and if one occurs, it reloads the file. But inside the ReloadFile code, we're sorting and saving, which causes this code to fire another ReloadFile.

A second work around I found was to alter the thread sleep time in FileChangeOvserver.cs, line 61. If I drop that to 500 instead of 1000, I don't see the issue.

[mjdescy]

@Piscean That's so much for catching this. I finally got a chance to look at it tonight.

It seems to me that it would be best to reduce the sleep time in FileChangeOvserver.cs line 61, as you suggest. Another thing to try might be to temporarily disable the FileChangeOvserver in the middle of batch updates and deletions.

I also want to reduce some of the calls to ReloadTasks() that occur when deleting tasks. I don't like how the file reloads over and over again each time a task is deleted. It makes archiving very slow.

[Piscean]

I had a few minutes to look at this again today. I dropped the time in FileChangeOvserver.cs line 61 to 250ms to see what would happen. At that setting I'm missing almost all changes made from another computer, so that tells me dropping this time probably isn't a safe approach on slower hardware.

I took a slightly different approach, based on your comment, and altered FileChange to this: private void FileChange(object source, FileSystemEventArgs e) { _watcher.EnableRaisingEvents = false; Thread.Sleep(1000); // give the writing app time to release its lock

        if (OnFileChanged != null)
            OnFileChanged();

        _watcher.EnableRaisingEvents = true;   
    }

That helped, but I still have 1 focus reset every time I delete a task. Better than the 5 or 6 I was seeing, but it's still there.

One more interesting note is ToggleCompletion() in MainWindowViewModel.cs, line 668. ModifySelectedTasks saves the todo.txt file. If AutoArchive is turned on, ArchiveCompleted also saves the todo.txt file. So we're saving the file twice 2 lines apart.

I haven't been able to determine whether these 2 file writes are happening before or after I set _watcher.EnableRaisingEvents = true;

[mjdescy]

When I tried changing the Thread.Sleep time on my build, it didn't eliminate the race condition, so I went in another direction.

I am working on a larger patch to address numerous issues with the FileChangeObserver. The changes include:

1. Temporarily disable the FileChangeObserver during updates that come from within the app. I don't think we need to be notified that Todotxt.net changed the file. It just causes extra reloads and saves. This seems to be the most important change.
2. Clean up how the FileChangeObserver is instantiated, so that it can be turned on and off--either in the Options window or programmatically during tasklist changes--without requiring an app restart.
3. Beneath the hood, in the TaskList class, remove the calls to the ReloadTasks() method within the methods for updates and deletions. Instead, move those calls to the MainWindowViewModel class, so we can explicitly refresh the tasklist only when we need to (before and after the batch update, rather than after every task is updated).

I haven't gotten all the bugs out, so I haven't committed anything to the repo yet.

[Piscean]

Sounds like good changes. Thanks.

[mjdescy]

@Piscean Please run a test build with my latest patch. I'd appreciate the help testing!

[Piscean]

The list loses focus when I delete a task. Repro steps:

1. With 8 or 9 items in the list, press X to delete the current item with focus.
2. Focus flashes to the list, and then vanishes. At this point none of the keyboard commands work. (a, x up/down arrows, etc)

When adding a new item, after pressing Enter to add the item, if I press the down arrow (twice per second), the selection starts at item #1, then moves down to 2 or 3, jumps back to #1, and then list loses focus as described above.

[mjdescy]

@Piscean I am seeing the focus loss after deleting tasks, too. I will look into it.

I think the second issue you wrote about has the same root cause.

[mjdescy]

@Piscean I don't understand it, but when I build and run the app from Visual Studio 2013, focus isn't lost after deleting tasks. When I run the executable in the Client > bin > Debug folder, though, focus is lost after deleting tasks.

[Piscean]

I see the loss of focus regardless of whether I run from within VS or from the Release folder. I'm running VS 2010, so maybe that's the difference.

If I turn Auto Archive off, it doesn't lose focus. But then as soon as I press 'A' to Archive, the completed tasks vanish and I lose focus.

[mjdescy]

The archive method calls the delete method, so I think the fixing the delete method, or whatever refreshes the list box after the delete method fires, may fix this entire issue. It is tricky. I will keep looking into it.

[mjdescy]

I narrowed down where the loss of focus is coming from. Focus is lost when you delete tasks (either directly or as part of an archive command) when the task list has groups displayed. This means, to reproduce the bug, the task list must be sorted ("Order in file" and "Alphabetical" don't count) and the "Allow grouping of tasks" option must be turned on.

Sorting and grouping is handled by the MainWindowViewModel method UpdateDisplayedTasks(). When the grouping-related code in that method is called after deleting tasks, focus to the listbox is lost, probably because the selected items have been removed from the list, and the top item in the listbox becomes an non-selectable group heading. Focus shouldn't be set to the listbox in that method, however, because it can be called even when the user doesn't have focus on the listbox (for example, when automatically reloading the file).

[mjdescy]

@Piscean The last two commits help fix the focus issues with the delete and archive methods. Please run a new test build. Thanks!

[Piscean]

That looks great! I'm not seeing the line 1 focus reset, or the loss of focus.

One thing I do notice we've lost is before version 3.0.0.0 if you deleted an item, the focus moved to the item next to the deleted one in the list. Now it jumps to item #1. Also, before, if you edited an item the focus stayed on that item after the edit. Now it jumps to first item in the list. Not the end of the world, but it is a change we should look at down the road.

I think your fixes have the focus issue solved.

[mjdescy]

Yes, there is still a little way to go in terms of making focus changes make sense after deleting tasks, and apparently after adding and updating them, too. I will continue to look into it.

[Piscean]

I've seen a few times focus is not returning correctly. I'll keep track of the cases where I see it get lost over the next couple days.

[mjdescy]

I'm starting to see a pattern (in the latest dev version) with the focus not returning correctly when I update tasks, only when "Automatically refresh task list from file" is checked. Also, the focus does not move to the newly-created or updated task, and that seems like a better choice than the current default, which is the top of the list.

[Piscean]

Changing filters causes the loss of focus as well. If I flip through 1, 2 & 3, I have to reset focus to the list between each filter. And yes, I have "Automatically refresh" set as one of my options.

[mjdescy]

Based on the commits I made tonight, I'd like to close this issue, if there is no dissent.

I know there are still some problems with focus and selection, but I think the main ones are now solved. I think that the remaining one is that, if there are duplicate tasks and only one or some are selected, the exact tasks selected will not necessarily be selected after a refresh. This behavior is due to the way tasks have no index (see Issue #158).

If other focus/selection-related issues come up, new issues can be opened.