From 3b7e52a4796bb45fa301291450951483a7bb59ed Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Thu, 4 Feb 2021 19:59:24 +0000 Subject: [PATCH 1/4] Fix issue with Focus call crashing Fixes #1893 --- internal/app/focus_manager.go | 12 ++++++++++++ internal/driver/glfw/canvas.go | 13 ++++++++++++- internal/driver/glfw/canvas_test.go | 11 +++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/internal/app/focus_manager.go b/internal/app/focus_manager.go index 1eff4bb605..668aa12520 100644 --- a/internal/app/focus_manager.go +++ b/internal/app/focus_manager.go @@ -59,6 +59,18 @@ func (f *FocusManager) Focus(obj fyne.Focusable) bool { return true } +// FocusBeforeAdded allows an object to be focused before it is added to the object tree. +// This is typically used before a canvas is visible and should be used with care. +func (f *FocusManager) FocusBeforeAdded(obj fyne.Focusable) { + f.RLock() + defer f.RUnlock() + if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { + return + } + + f.focus(obj) +} + // Focused returns the currently focused object or nil if none. func (f *FocusManager) Focused() fyne.Focusable { f.RLock() diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 265763cc3f..8f46a65e49 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -84,13 +84,17 @@ func (c *glCanvas) Focus(obj fyne.Focusable) { c.RUnlock() for _, mgr := range focusMgrs { + if mgr == nil { + continue + } if focusMgr != mgr { if mgr.Focus(obj) { return } } } - fyne.LogError("Failed to focus object which is not part of the canvas’ content, menu or overlays.", nil) + + c.contentFocusMgr.FocusBeforeAdded(obj) // not found yet assume we are preparing the UI } func (c *glCanvas) Focused() fyne.Focusable { @@ -452,7 +456,14 @@ func (c *glCanvas) paint(size fyne.Size) { func (c *glCanvas) setContent(content fyne.CanvasObject) { c.content = content c.contentTree = &renderCacheTree{root: &renderCacheNode{obj: c.content}} + var focused fyne.Focusable + if c.contentFocusMgr != nil { + focused = c.contentFocusMgr.Focused() // keep old focus if possible + } c.contentFocusMgr = app.NewFocusManager(c.content) + if focused != nil { + c.contentFocusMgr.Focus(focused) + } } func (c *glCanvas) setDirty(dirty bool) { diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index f5f6ccfe0e..8189b73f38 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -204,6 +204,17 @@ func TestGlCanvas_Focus(t *testing.T) { assert.True(t, o2e.focused) } +func TestGlCanvas_Focus_BeforeVisible(t *testing.T) { + w := createWindow("Test") + w.SetPadded(false) + e := widget.NewEntry() + c := w.Canvas().(*glCanvas) + c.Focus(e) // this crashed in the past + + w.SetContent(e) + assert.Equal(t, e, c.Focused(), "Item set to focus before SetContent was not focused after") +} + func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) { w := createWindow("Test") w.SetPadded(false) From cdf85b6633dd43a3f16be9c511f5e7bb4c0e1638 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Fri, 12 Feb 2021 22:35:19 +0000 Subject: [PATCH 2/4] Roll back the ability to focus elements not yet in tree --- internal/app/focus_manager.go | 12 ------------ internal/driver/glfw/canvas.go | 2 +- internal/driver/glfw/canvas_test.go | 3 --- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/internal/app/focus_manager.go b/internal/app/focus_manager.go index 668aa12520..1eff4bb605 100644 --- a/internal/app/focus_manager.go +++ b/internal/app/focus_manager.go @@ -59,18 +59,6 @@ func (f *FocusManager) Focus(obj fyne.Focusable) bool { return true } -// FocusBeforeAdded allows an object to be focused before it is added to the object tree. -// This is typically used before a canvas is visible and should be used with care. -func (f *FocusManager) FocusBeforeAdded(obj fyne.Focusable) { - f.RLock() - defer f.RUnlock() - if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { - return - } - - f.focus(obj) -} - // Focused returns the currently focused object or nil if none. func (f *FocusManager) Focused() fyne.Focusable { f.RLock() diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 8f46a65e49..3db030c6f7 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -94,7 +94,7 @@ func (c *glCanvas) Focus(obj fyne.Focusable) { } } - c.contentFocusMgr.FocusBeforeAdded(obj) // not found yet assume we are preparing the UI + fyne.LogError("Failed to focus object which is not part of the canvas’ content, menu or overlays.", nil) } func (c *glCanvas) Focused() fyne.Focusable { diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 8189b73f38..739a528a4d 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -210,9 +210,6 @@ func TestGlCanvas_Focus_BeforeVisible(t *testing.T) { e := widget.NewEntry() c := w.Canvas().(*glCanvas) c.Focus(e) // this crashed in the past - - w.SetContent(e) - assert.Equal(t, e, c.Focused(), "Item set to focus before SetContent was not focused after") } func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) { From ea49789d23a5be19be0e105ca9191dc1e8a36598 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 15 Feb 2021 10:04:03 +0000 Subject: [PATCH 3/4] Add missing test --- internal/driver/glfw/canvas_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 739a528a4d..36fcd58f0b 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -212,6 +212,19 @@ func TestGlCanvas_Focus_BeforeVisible(t *testing.T) { c.Focus(e) // this crashed in the past } +func TestGlCanvas_Focus_SetContent(t *testing.T) { + w := createWindow("Test") + w.SetPadded(false) + e := widget.NewEntry() + w.SetContent(e) + c := w.Canvas().(*glCanvas) + c.Focus(e) + assert.Equal(t, e, c.Focused()) + + w.SetContent(e) + assert.Equal(t, e, c.Focused()) +} + func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) { w := createWindow("Test") w.SetPadded(false) From 30d6fc0ea716a983425a285a4c9ceac78136ab79 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 15 Feb 2021 21:00:03 +0000 Subject: [PATCH 4/4] Update internal/driver/glfw/canvas_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tilo Prütz --- internal/driver/glfw/canvas_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 36fcd58f0b..fae107ec97 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -216,12 +216,12 @@ func TestGlCanvas_Focus_SetContent(t *testing.T) { w := createWindow("Test") w.SetPadded(false) e := widget.NewEntry() - w.SetContent(e) + w.SetContent(container.NewHBox(e)) c := w.Canvas().(*glCanvas) c.Focus(e) assert.Equal(t, e, c.Focused()) - w.SetContent(e) + w.SetContent(container.NewVBox(e)) assert.Equal(t, e, c.Focused()) }