Merge pull request #1318 from toaster/bugfix/948

Bugfix #948: support focus traversal in dialogs

canvasobject.go

@@ -66,6 +66,9 @@ type Draggable interface {
 // It will receive the FocusGained and FocusLost events appropriately.
 // When focused it will also have TypedRune called as text is input and
 // TypedKey called when other keys are pressed.
+//
+// Note: You must not change canvas state (including overlays or focus) in FocusGained or FocusLost
+// or you would end up with a dead-lock.
 type Focusable interface {
 	FocusGained()
 	FocusLost()

internal/driver/glfw/canvas.go

@@ -112,9 +112,7 @@ func (c *glCanvas) OnTypedRune() func(rune) {
 
 // Deprecated: Use Overlays() instead.
 func (c *glCanvas) Overlay() fyne.CanvasObject {
-	c.RLock()
-	defer c.RUnlock()
-	return c.overlays.Top()
+	return c.Overlays().Top()
 }
 
 func (c *glCanvas) Overlays() fyne.OverlayStack {
@@ -210,13 +208,10 @@ func (c *glCanvas) SetOnTypedRune(typed func(rune)) {
 
 // Deprecated: Use Overlays() instead.
 func (c *glCanvas) SetOverlay(overlay fyne.CanvasObject) {
-	c.Lock()
-	defer c.Unlock()
-	if len(c.overlays.List()) > 0 {
-		c.overlays.Remove(c.overlays.List()[0])
-	}
-	c.overlays.Add(overlay)
-	c.setDirty(true)
+	c.RLock()
+	o := c.overlays
+	c.RUnlock()
+	o.setOverlay(overlay)
 }
 
 func (c *glCanvas) SetPadded(padded bool) {
@@ -356,6 +351,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
 }
 
@@ -390,6 +388,20 @@ func (c *glCanvas) objectTrees() []fyne.CanvasObject {
 	return trees
 }
 
+func (c *glCanvas) overlayChanged(focusMgr, prevFocusMgr *app.FocusManager) {
+	c.Lock()
+	defer c.Unlock()
+	c.dirty = true
+	if prevFocusMgr == nil {
+		prevFocusMgr = c.focusMgr
+	}
+	if focusMgr == nil {
+		focusMgr = c.focusMgr
+	}
+	prevFocusMgr.FocusLost()
+	focusMgr.FocusGained()
+}
+
 func (c *glCanvas) paint(size fyne.Size) {
 	if c.Content() == nil {
 		return
@@ -524,23 +536,74 @@ func (c *glCanvas) walkTrees(
 type overlayStack struct {
 	internal.OverlayStack
 
-	onChange     func()
-	renderCaches []*renderCacheTree
+	focusManagers []*app.FocusManager
+	onChange      func(focusMgr, prevFocusMgr *app.FocusManager)
+	propertyLock  sync.RWMutex
+	renderCaches  []*renderCacheTree
 }
 
 func (o *overlayStack) Add(overlay fyne.CanvasObject) {
 	if overlay == nil {
 		return
 	}
+	o.propertyLock.Lock()
+	defer o.propertyLock.Unlock()
+	pfm := o.topFocusManager()
+	o.add(overlay)
+	o.onChange(o.topFocusManager(), pfm)
+}
+
+func (o *overlayStack) Remove(overlay fyne.CanvasObject) {
+	if overlay == nil || len(o.List()) == 0 {
+		return
+	}
+	o.propertyLock.Lock()
+	defer o.propertyLock.Unlock()
+	pfm := o.topFocusManager()
+	o.remove(overlay)
+	o.onChange(o.topFocusManager(), pfm)
+}
+
+func (o *overlayStack) TopFocusManager() *app.FocusManager {
+	o.propertyLock.RLock()
+	defer o.propertyLock.RUnlock()
+	return o.topFocusManager()
+}
+
+func (o *overlayStack) add(overlay fyne.CanvasObject) {
 	o.OverlayStack.Add(overlay)
 	o.renderCaches = append(o.renderCaches, &renderCacheTree{root: &renderCacheNode{obj: overlay}})
-	o.onChange()
+	o.focusManagers = append(o.focusManagers, app.NewFocusManager(overlay))
 }
 
-func (o *overlayStack) Remove(overlay fyne.CanvasObject) {
+func (o *overlayStack) remove(overlay fyne.CanvasObject) {
 	o.OverlayStack.Remove(overlay)
-	o.renderCaches = o.renderCaches[:len(o.List())]
-	o.onChange()
+	overlayCount := len(o.List())
+	o.renderCaches = o.renderCaches[:overlayCount]
+	o.focusManagers = o.focusManagers[:overlayCount]
+}
+
+// concurrency safe implementation of deprecated c.SetOverlay
+func (o *overlayStack) setOverlay(overlay fyne.CanvasObject) {
+	o.propertyLock.Lock()
+	defer o.propertyLock.Unlock()
+
+	pfm := o.topFocusManager()
+	if len(o.List()) > 0 {
+		o.remove(o.List()[0])
+	}
+	if overlay != nil {
+		o.add(overlay)
+	}
+	o.onChange(o.topFocusManager(), pfm)
+}
+
+func (o *overlayStack) topFocusManager() *app.FocusManager {
+	var fm *app.FocusManager
+	if len(o.focusManagers) > 0 {
+		fm = o.focusManagers[len(o.focusManagers)-1]
+	}
+	return fm
 }
 
 type renderCacheNode struct {
@@ -568,7 +631,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{}