fix fyne-io#948: delegate focus handling for overlays to correct manager

internal/driver/glfw/canvas.go

@@ -352,6 +352,9 @@ func (c *glCanvas) ensureMinSize() bool {
 func (c *glCanvas) focusManager() *app.FocusManager {
 	c.RLock()
 	defer c.RUnlock()
+	if focusMgr := c.overlays.TopFocusManager(); focusMgr != nil {
+		return focusMgr
+	}
 	return c.focusMgr
 }
 
@@ -386,6 +389,21 @@ func (c *glCanvas) objectTrees() []fyne.CanvasObject {
 	return trees
 }
 
+func (c *glCanvas) overlayChanged(prevFocusMgr *app.FocusManager) {
+	c.setDirty(true)
+	c.RLock()
+	if prevFocusMgr == nil {
+		prevFocusMgr = c.focusMgr
+	}
+	focusMgr := c.overlays.TopFocusManager()
+	if focusMgr == nil {
+		focusMgr = c.focusMgr
+	}
+	c.RUnlock()
+	prevFocusMgr.FocusLost()
+	focusMgr.FocusGained()
+}
+
 func (c *glCanvas) paint(size fyne.Size) {
 	if c.Content() == nil {
 		return
@@ -521,26 +539,36 @@ type overlayStack struct {
 	internal.OverlayStack
 
 	focusManagers []*app.FocusManager
-	onChange      func()
+	onChange      func(prevFocusMgr *app.FocusManager)
 	renderCaches  []*renderCacheTree
 }
 
 func (o *overlayStack) Add(overlay fyne.CanvasObject) {
 	if overlay == nil {
 		return
 	}
+	pfm := o.TopFocusManager()
 	o.OverlayStack.Add(overlay)
 	o.renderCaches = append(o.renderCaches, &renderCacheTree{root: &renderCacheNode{obj: overlay}})
 	o.focusManagers = append(o.focusManagers, app.NewFocusManager(overlay))
-	o.onChange()
+	o.onChange(pfm)
 }
 
 func (o *overlayStack) Remove(overlay fyne.CanvasObject) {
+	top := o.TopFocusManager()
 	o.OverlayStack.Remove(overlay)
 	overlayCount := len(o.List())
 	o.renderCaches = o.renderCaches[:overlayCount]
 	o.focusManagers = o.focusManagers[:overlayCount]
-	o.onChange()
+	o.onChange(top)
+}
+
+func (o *overlayStack) TopFocusManager() *app.FocusManager {
+	var pfm *app.FocusManager
+	if len(o.focusManagers) > 0 {
+		pfm = o.focusManagers[len(o.focusManagers)-1]
+	}
+	return pfm
 }
 
 type renderCacheNode struct {
@@ -568,7 +596,7 @@ func newCanvas() *glCanvas {
 	c.setContent(&canvas.Rectangle{FillColor: theme.BackgroundColor()})
 	c.padded = true
 
-	c.overlays = &overlayStack{onChange: func() { c.setDirty(true) }}
+	c.overlays = &overlayStack{onChange: c.overlayChanged}
 
 	c.refreshQueue = make(chan fyne.CanvasObject, 4096)
 	c.dirtyMutex = &sync.Mutex{}

internal/driver/glfw/canvas_test.go

@@ -9,6 +9,7 @@ import (
 
 	"fyne.io/fyne"
 	"fyne.io/fyne/canvas"
+	"fyne.io/fyne/layout"
 	"fyne.io/fyne/theme"
 	"fyne.io/fyne/widget"
 
@@ -164,6 +165,77 @@ func TestGlCanvas_ContentChangeWithoutMinSizeChangeDoesNotLayout(t *testing.T) {
 	assert.Nil(t, layout.popLayoutEvent())
 }
 
+func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) {
+	w := createWindow("Test")
+	w.SetPadded(false)
+	c := w.Canvas().(*glCanvas)
+
+	ce1 := &focusable{id: "ce1"}
+	ce2 := &focusable{id: "ce2"}
+	content := layout.NewVBoxContainer(ce1, ce2)
+	o1e1 := &focusable{id: "o1e1"}
+	o1e2 := &focusable{id: "o1e2"}
+	overlay1 := layout.NewVBoxContainer(o1e1, o1e2)
+	o2e1 := &focusable{id: "o2e1"}
+	o2e2 := &focusable{id: "o2e2"}
+	overlay2 := layout.NewVBoxContainer(o2e1, o2e2)
+	w.SetContent(content)
+
+	assert.Nil(t, c.Focused())
+
+	c.FocusPrevious()
+	assert.Equal(t, ce2, c.Focused())
+	assert.True(t, ce2.focused)
+
+	c.Overlays().Add(overlay1)
+	ctxt := "adding overlay removes focus from content"
+	assert.Nil(t, c.Focused(), ctxt)
+	assert.False(t, ce2.focused, ctxt)
+
+	c.FocusNext()
+	ctxt = "changing focus affects overlay instead of content"
+	assert.Equal(t, o1e1, c.Focused(), ctxt)
+	assert.False(t, ce1.focused, ctxt)
+	assert.False(t, ce2.focused, ctxt)
+	assert.True(t, o1e1.focused, ctxt)
+
+	c.Overlays().Add(overlay2)
+	ctxt = "adding overlay removes focus from previous overlay"
+	assert.Nil(t, c.Focused(), ctxt)
+	assert.False(t, o1e1.focused, ctxt)
+
+	c.FocusPrevious()
+	ctxt = "changing focus affects top overlay only"
+	assert.Equal(t, o2e2, c.Focused(), ctxt)
+	assert.False(t, o1e1.focused, ctxt)
+	assert.False(t, o1e2.focused, ctxt)
+	assert.True(t, o2e2.focused, ctxt)
+
+	c.FocusNext()
+	assert.Equal(t, o2e1, c.Focused())
+	assert.False(t, o2e2.focused)
+	assert.True(t, o2e1.focused)
+
+	c.Overlays().Remove(overlay2)
+	ctxt = "removing overlay removes focus from removed overlay and restores focus on new top overlay"
+	assert.Equal(t, o1e1, c.Focused(), ctxt)
+	assert.False(t, o2e1.focused, ctxt)
+	assert.False(t, o2e2.focused, ctxt)
+	assert.True(t, o1e1.focused, ctxt)
+
+	c.FocusPrevious()
+	assert.Equal(t, o1e2, c.Focused())
+	assert.False(t, o1e1.focused)
+	assert.True(t, o1e2.focused)
+
+	c.Overlays().Remove(overlay1)
+	ctxt = "removing last overlay removes focus from removed overlay and restores focus on content"
+	assert.Equal(t, ce2, c.Focused(), ctxt)
+	assert.False(t, o1e1.focused, ctxt)
+	assert.False(t, o1e2.focused, ctxt)
+	assert.True(t, ce2.focused, ctxt)
+}
+
 func TestGlCanvas_InsufficientSizeDoesntTriggerResizeIfSizeIsAlreadyMaxedOut(t *testing.T) {
 	w := createWindow("Test").(*window)
 	c := w.Canvas().(*glCanvas)

internal/driver/glfw/window_test.go

@@ -778,7 +778,7 @@ func TestWindow_Focus(t *testing.T) {
 
 func TestWindow_ManualFocus(t *testing.T) {
 	w := createWindow("Test").(*window)
-	content := &focusable{Rectangle: canvas.NewRectangle(color.Black)}
+	content := &focusable{}
 	content.SetMinSize(fyne.NewSize(10, 10))
 	w.SetContent(content)
 	repaintWindow(w)
@@ -1035,7 +1035,9 @@ var _ fyne.Focusable = (*focusable)(nil)
 var _ fyne.Disableable = (*focusable)(nil)
 
 type focusable struct {
-	*canvas.Rectangle
+	canvas.Rectangle
+	id             string // helps identifying instances in comparisons
+	focused        bool
 	focusedTimes   int
 	unfocusedTimes int
 	disabled       bool
@@ -1053,10 +1055,15 @@ func (f *focusable) TypedKey(*fyne.KeyEvent) {
 
 func (f *focusable) FocusGained() {
 	f.focusedTimes++
+	if f.Disabled() {
+		return
+	}
+	f.focused = true
 }
 
 func (f *focusable) FocusLost() {
 	f.unfocusedTimes++
+	f.focused = false
 }
 
 func (f *focusable) Enable() {