fix(cef): bind permission dialog for every permission request
The dialog presenter was only bound from the WebRTC activity hook, so geolocation/notification-only requests had no dialog and were denied, and multi-window setups could bind the wrong window. Add a coordinator hook invoked before every permission request and bind the pane's window there.
- commit
- 664e13ed3c6ae7ef5cedb61549ffce8343f25c80
- author
- brice · · 2026-10-03 06:20 UTC
- committer
- brice · · 2026-10-03 07:50 UTC
- parent
- ad51442
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +86 -3
M internal/ui/app.go +9 -3
| @@ -3598,6 +3598,15 @@ func (a *App) wireWebRTCPermissionIndicator() { | |||
| 3598 | 3598 | ctx = a.deps.Ctx | |
| 3599 | 3599 | } | |
| 3600 | 3600 | ||
| 3601 | + | a.contentCoord.SetOnPermissionPrompt(func(paneID entity.PaneID) { | |
| 3602 | + | if a.deps == nil || a.deps.PermissionUC == nil { | |
| 3603 | + | return | |
| 3604 | + | } | |
| 3605 | + | if bw := a.browserWindowForPane(paneID); bw != nil && bw.permissionDialog != nil { | |
| 3606 | + | a.deps.PermissionUC.SetDialogPresenter(bw.permissionDialog) | |
| 3607 | + | } | |
| 3608 | + | }) | |
| 3609 | + | ||
| 3601 | 3610 | a.contentCoord.SetOnPermissionActivity(func( | |
| 3602 | 3611 | paneID entity.PaneID, | |
| 3603 | 3612 | origin string, | |
| @@ -3608,9 +3617,6 @@ func (a *App) wireWebRTCPermissionIndicator() { | |||
| 3608 | 3617 | if bw == nil || bw.webrtcIndicator == nil { | |
| 3609 | 3618 | return | |
| 3610 | 3619 | } | |
| 3611 | - | if state == content.PermissionActivityRequesting && a.deps != nil && a.deps.PermissionUC != nil && bw.permissionDialog != nil { | |
| 3612 | - | a.deps.PermissionUC.SetDialogPresenter(bw.permissionDialog) | |
| 3613 | - | } | |
| 3614 | 3620 | bw.webrtcIndicator.SetOrigin(origin) | |
| 3615 | 3621 | ||
| 3616 | 3622 | switch state { | |
| @@ -3598,6 +3598,15 @@ func (a *App) wireWebRTCPermissionIndicator() { | |||||
| 3598 | ctx = a.deps.Ctx | 3598 | ctx = a.deps.Ctx | ||
| 3599 | } | 3599 | } | ||
| 3600 | 3600 | ||||
| 3601 | + | a.contentCoord.SetOnPermissionPrompt(func(paneID entity.PaneID) { | |||
| 3602 | + | if a.deps == nil || a.deps.PermissionUC == nil { | |||
| 3603 | + | return | |||
| 3604 | + | } | |||
| 3605 | + | if bw := a.browserWindowForPane(paneID); bw != nil && bw.permissionDialog != nil { | |||
| 3606 | + | a.deps.PermissionUC.SetDialogPresenter(bw.permissionDialog) | |||
| 3607 | + | } | |||
| 3608 | + | }) | |||
| 3609 | + | ||||
| 3601 | a.contentCoord.SetOnPermissionActivity(func( | 3610 | a.contentCoord.SetOnPermissionActivity(func( | ||
| 3602 | paneID entity.PaneID, | 3611 | paneID entity.PaneID, | ||
| 3603 | origin string, | 3612 | origin string, | ||
| @@ -3608,9 +3617,6 @@ func (a *App) wireWebRTCPermissionIndicator() { | |||||
| 3608 | if bw == nil || bw.webrtcIndicator == nil { | 3617 | if bw == nil || bw.webrtcIndicator == nil { | ||
| 3609 | return | 3618 | return | ||
| 3610 | } | 3619 | } | ||
| 3611 | - | if state == content.PermissionActivityRequesting && a.deps != nil && a.deps.PermissionUC != nil && bw.permissionDialog != nil { | |||
| 3612 | - | a.deps.PermissionUC.SetDialogPresenter(bw.permissionDialog) | |||
| 3613 | - | } | |||
| 3614 | bw.webrtcIndicator.SetOrigin(origin) | 3620 | bw.webrtcIndicator.SetOrigin(origin) | ||
| 3615 | 3621 | ||||
| 3616 | switch state { | 3622 | switch state { | ||
M internal/ui/coordinator/content/callbacks.go +5 -0
| @@ -270,6 +270,11 @@ func (c *Coordinator) handlePermissionRequest( | |||
| 270 | 270 | return true | |
| 271 | 271 | } | |
| 272 | 272 | ||
| 273 | + | // Bind the dialog presenter of the window owning this pane before handling. | |
| 274 | + | if c.onPermissionPrompt != nil { | |
| 275 | + | c.onPermissionPrompt(paneID) | |
| 276 | + | } | |
| 277 | + | ||
| 273 | 278 | // Delegate to use case | |
| 274 | 279 | callback := usecase.PermissionCallback{ | |
| 275 | 280 | Allow: wrappedAllow, | |
| @@ -270,6 +270,11 @@ func (c *Coordinator) handlePermissionRequest( | |||||
| 270 | return true | 270 | return true | ||
| 271 | } | 271 | } | ||
| 272 | 272 | ||||
| 273 | + | // Bind the dialog presenter of the window owning this pane before handling. | |||
| 274 | + | if c.onPermissionPrompt != nil { | |||
| 275 | + | c.onPermissionPrompt(paneID) | |||
| 276 | + | } | |||
| 277 | + | ||||
| 273 | // Delegate to use case | 278 | // Delegate to use case | ||
| 274 | callback := usecase.PermissionCallback{ | 279 | callback := usecase.PermissionCallback{ | ||
| 275 | Allow: wrappedAllow, | 280 | Allow: wrappedAllow, | ||
M internal/ui/coordinator/content/coordinator.go +10 -0
| @@ -61,6 +61,10 @@ type Coordinator struct { | |||
| 61 | 61 | // Callback when media permission activity changes (requesting/allowed/blocked). | |
| 62 | 62 | onPermissionActivity func(paneID entity.PaneID, origin string, permTypes []entity.PermissionType, state PermissionActivityState) | |
| 63 | 63 | ||
| 64 | + | // Callback invoked for every permission request, before it is handled, so the | |
| 65 | + | // UI can bind the permission dialog of the window owning the pane. | |
| 66 | + | onPermissionPrompt func(paneID entity.PaneID) | |
| 67 | + | ||
| 64 | 68 | // Callback when the active pane commits a navigation (new page loading). | |
| 65 | 69 | onActiveNavigationCommitted func(paneID entity.PaneID, uri string) | |
| 66 | 70 | ||
| @@ -200,6 +204,12 @@ func (c *Coordinator) SetOnPermissionActivity( | |||
| 200 | 204 | c.onPermissionActivity = fn | |
| 201 | 205 | } | |
| 202 | 206 | ||
| 207 | + | // SetOnPermissionPrompt sets a callback invoked for every permission request | |
| 208 | + | // (not only WebRTC ones) before it reaches the permission use case. | |
| 209 | + | func (c *Coordinator) SetOnPermissionPrompt(fn func(paneID entity.PaneID)) { | |
| 210 | + | c.onPermissionPrompt = fn | |
| 211 | + | } | |
| 212 | + | ||
| 203 | 213 | // SetOnActiveNavigationCommitted sets a callback fired when the active pane commits a navigation. | |
| 204 | 214 | func (c *Coordinator) SetOnActiveNavigationCommitted(fn func(paneID entity.PaneID, uri string)) { | |
| 205 | 215 | c.onActiveNavigationCommitted = fn | |
| @@ -61,6 +61,10 @@ type Coordinator struct { | |||||
| 61 | // Callback when media permission activity changes (requesting/allowed/blocked). | 61 | // Callback when media permission activity changes (requesting/allowed/blocked). | ||
| 62 | onPermissionActivity func(paneID entity.PaneID, origin string, permTypes []entity.PermissionType, state PermissionActivityState) | 62 | onPermissionActivity func(paneID entity.PaneID, origin string, permTypes []entity.PermissionType, state PermissionActivityState) | ||
| 63 | 63 | ||||
| 64 | + | // Callback invoked for every permission request, before it is handled, so the | |||
| 65 | + | // UI can bind the permission dialog of the window owning the pane. | |||
| 66 | + | onPermissionPrompt func(paneID entity.PaneID) | |||
| 67 | + | ||||
| 64 | // Callback when the active pane commits a navigation (new page loading). | 68 | // Callback when the active pane commits a navigation (new page loading). | ||
| 65 | onActiveNavigationCommitted func(paneID entity.PaneID, uri string) | 69 | onActiveNavigationCommitted func(paneID entity.PaneID, uri string) | ||
| 66 | 70 | ||||
| @@ -200,6 +204,12 @@ func (c *Coordinator) SetOnPermissionActivity( | |||||
| 200 | c.onPermissionActivity = fn | 204 | c.onPermissionActivity = fn | ||
| 201 | } | 205 | } | ||
| 202 | 206 | ||||
| 207 | + | // SetOnPermissionPrompt sets a callback invoked for every permission request | |||
| 208 | + | // (not only WebRTC ones) before it reaches the permission use case. | |||
| 209 | + | func (c *Coordinator) SetOnPermissionPrompt(fn func(paneID entity.PaneID)) { | |||
| 210 | + | c.onPermissionPrompt = fn | |||
| 211 | + | } | |||
| 212 | + | ||||
| 203 | // SetOnActiveNavigationCommitted sets a callback fired when the active pane commits a navigation. | 213 | // SetOnActiveNavigationCommitted sets a callback fired when the active pane commits a navigation. | ||
| 204 | func (c *Coordinator) SetOnActiveNavigationCommitted(fn func(paneID entity.PaneID, uri string)) { | 214 | func (c *Coordinator) SetOnActiveNavigationCommitted(fn func(paneID entity.PaneID, uri string)) { | ||
| 205 | c.onActiveNavigationCommitted = fn | 215 | c.onActiveNavigationCommitted = fn | ||
A internal/ui/coordinator/content/permission_prompt_test.go +62 -0
| @@ -0,0 +1,62 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/rs/zerolog" | |
| 8 | + | "github.com/stretchr/testify/assert" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ||
| 12 | + | "github.com/bnema/dumber/internal/application/port" | |
| 13 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 14 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 15 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 16 | + | ) | |
| 17 | + | ||
| 18 | + | func TestHandlePermissionRequest_BindsDialogBeforeUseCaseForNonWebRTCRequest(t *testing.T) { | |
| 19 | + | permRepo := portmocks.NewMockPermissionRepository(t) | |
| 20 | + | dialog := portmocks.NewMockPermissionDialogPresenter(t) | |
| 21 | + | // The use case starts without a dialog presenter, like in production. | |
| 22 | + | uc := usecase.NewHandlePermissionUseCase(permRepo, nil, func(ctx context.Context) *zerolog.Logger { | |
| 23 | + | return zerolog.Ctx(ctx) | |
| 24 | + | }) | |
| 25 | + | ||
| 26 | + | permRepo.EXPECT().Get(mock.Anything, "https://maps.example.com", entity.PermissionTypeGeolocation). | |
| 27 | + | Return(nil, nil) | |
| 28 | + | ||
| 29 | + | var order []string | |
| 30 | + | dialog.EXPECT(). | |
| 31 | + | ShowPermissionDialog(mock.Anything, "https://maps.example.com", []entity.PermissionType{entity.PermissionTypeGeolocation}, mock.Anything, mock.Anything). | |
| 32 | + | Run(func(_ context.Context, _ string, _ []entity.PermissionType, _ entity.PermissionMetadata, onResult func(port.PermissionDialogResult)) { | |
| 33 | + | order = append(order, "dialog") | |
| 34 | + | onResult(port.PermissionDialogResult{Allowed: true}) | |
| 35 | + | }).Return() | |
| 36 | + | ||
| 37 | + | pane := entity.PaneID("pane-1") | |
| 38 | + | var gotPane entity.PaneID | |
| 39 | + | activityCalls := 0 | |
| 40 | + | c := &Coordinator{ | |
| 41 | + | permissionUC: uc, | |
| 42 | + | onPermissionPrompt: func(paneID entity.PaneID) { | |
| 43 | + | order = append(order, "bind") | |
| 44 | + | gotPane = paneID | |
| 45 | + | uc.SetDialogPresenter(dialog) | |
| 46 | + | }, | |
| 47 | + | onPermissionActivity: func(entity.PaneID, string, []entity.PermissionType, PermissionActivityState) { | |
| 48 | + | activityCalls++ | |
| 49 | + | }, | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | allowed, denied := 0, 0 | |
| 53 | + | handled := c.handlePermissionRequest(context.Background(), pane, "https://maps.example.com", | |
| 54 | + | []string{"geolocation"}, nil, func() { allowed++ }, func() { denied++ }) | |
| 55 | + | ||
| 56 | + | require.True(t, handled) | |
| 57 | + | assert.Equal(t, pane, gotPane) | |
| 58 | + | assert.Equal(t, []string{"bind", "dialog"}, order) | |
| 59 | + | assert.Equal(t, 1, allowed) | |
| 60 | + | assert.Zero(t, denied) | |
| 61 | + | assert.Zero(t, activityCalls, "geolocation is not tracked by the WebRTC indicator") | |
| 62 | + | } | |
| @@ -0,0 +1,62 @@ | |||||
| 1 | + | package content | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/rs/zerolog" | |||
| 8 | + | "github.com/stretchr/testify/assert" | |||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ||||
| 12 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 13 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 14 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 15 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 16 | + | ) | |||
| 17 | + | ||||
| 18 | + | func TestHandlePermissionRequest_BindsDialogBeforeUseCaseForNonWebRTCRequest(t *testing.T) { | |||
| 19 | + | permRepo := portmocks.NewMockPermissionRepository(t) | |||
| 20 | + | dialog := portmocks.NewMockPermissionDialogPresenter(t) | |||
| 21 | + | // The use case starts without a dialog presenter, like in production. | |||
| 22 | + | uc := usecase.NewHandlePermissionUseCase(permRepo, nil, func(ctx context.Context) *zerolog.Logger { | |||
| 23 | + | return zerolog.Ctx(ctx) | |||
| 24 | + | }) | |||
| 25 | + | ||||
| 26 | + | permRepo.EXPECT().Get(mock.Anything, "https://maps.example.com", entity.PermissionTypeGeolocation). | |||
| 27 | + | Return(nil, nil) | |||
| 28 | + | ||||
| 29 | + | var order []string | |||
| 30 | + | dialog.EXPECT(). | |||
| 31 | + | ShowPermissionDialog(mock.Anything, "https://maps.example.com", []entity.PermissionType{entity.PermissionTypeGeolocation}, mock.Anything, mock.Anything). | |||
| 32 | + | Run(func(_ context.Context, _ string, _ []entity.PermissionType, _ entity.PermissionMetadata, onResult func(port.PermissionDialogResult)) { | |||
| 33 | + | order = append(order, "dialog") | |||
| 34 | + | onResult(port.PermissionDialogResult{Allowed: true}) | |||
| 35 | + | }).Return() | |||
| 36 | + | ||||
| 37 | + | pane := entity.PaneID("pane-1") | |||
| 38 | + | var gotPane entity.PaneID | |||
| 39 | + | activityCalls := 0 | |||
| 40 | + | c := &Coordinator{ | |||
| 41 | + | permissionUC: uc, | |||
| 42 | + | onPermissionPrompt: func(paneID entity.PaneID) { | |||
| 43 | + | order = append(order, "bind") | |||
| 44 | + | gotPane = paneID | |||
| 45 | + | uc.SetDialogPresenter(dialog) | |||
| 46 | + | }, | |||
| 47 | + | onPermissionActivity: func(entity.PaneID, string, []entity.PermissionType, PermissionActivityState) { | |||
| 48 | + | activityCalls++ | |||
| 49 | + | }, | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | allowed, denied := 0, 0 | |||
| 53 | + | handled := c.handlePermissionRequest(context.Background(), pane, "https://maps.example.com", | |||
| 54 | + | []string{"geolocation"}, nil, func() { allowed++ }, func() { denied++ }) | |||
| 55 | + | ||||
| 56 | + | require.True(t, handled) | |||
| 57 | + | assert.Equal(t, pane, gotPane) | |||
| 58 | + | assert.Equal(t, []string{"bind", "dialog"}, order) | |||
| 59 | + | assert.Equal(t, 1, allowed) | |||
| 60 | + | assert.Zero(t, denied) | |||
| 61 | + | assert.Zero(t, activityCalls, "geolocation is not tracked by the WebRTC indicator") | |||
| 62 | + | } | |||