refactor(ui,webkit): apply review quality improvements
- commit
- d03767992bcbaf098679207826cee803744fcbaf
- author
- brice · · 2026-02-08 07:35 UTC
- parent
- c51ceb8
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
16 files · +155 -41
M internal/infrastructure/persistence/sqlite/migrations/007_permissions.sql +0 -1
| @@ -10,7 +10,6 @@ CREATE TABLE IF NOT EXISTS permissions ( | |||
| 10 | 10 | PRIMARY KEY (origin, permission_type) | |
| 11 | 11 | ); | |
| 12 | 12 | ||
| 13 | - | CREATE INDEX IF NOT EXISTS idx_permissions_origin ON permissions(origin); | |
| 14 | 13 | CREATE INDEX IF NOT EXISTS idx_permissions_updated_at ON permissions(updated_at); | |
| 15 | 14 | ||
| 16 | 15 | -- +goose Down | |
| @@ -10,7 +10,6 @@ CREATE TABLE IF NOT EXISTS permissions ( | |||||
| 10 | PRIMARY KEY (origin, permission_type) | 10 | PRIMARY KEY (origin, permission_type) | ||
| 11 | ); | 11 | ); | ||
| 12 | 12 | ||||
| 13 | - | CREATE INDEX IF NOT EXISTS idx_permissions_origin ON permissions(origin); | |||
| 14 | CREATE INDEX IF NOT EXISTS idx_permissions_updated_at ON permissions(updated_at); | 13 | CREATE INDEX IF NOT EXISTS idx_permissions_updated_at ON permissions(updated_at); | ||
| 15 | 14 | ||||
| 16 | -- +goose Down | 15 | -- +goose Down | ||
M internal/infrastructure/persistence/sqlite/permission_repo.go +4 -0
| @@ -73,10 +73,14 @@ func (r *permissionRepo) GetAll(ctx context.Context, origin string) ([]*entity.P | |||
| 73 | 73 | log := logging.FromContext(ctx) | |
| 74 | 74 | log.Debug().Str("origin", origin).Msg("getting all permissions for origin") | |
| 75 | 75 | ||
| 76 | + | // sqlc returns an empty slice (not sql.ErrNoRows) when nothing matches. | |
| 76 | 77 | rows, err := r.queries.ListPermissionsByOrigin(ctx, origin) | |
| 77 | 78 | if err != nil { | |
| 78 | 79 | return nil, err | |
| 79 | 80 | } | |
| 81 | + | if len(rows) == 0 { | |
| 82 | + | return []*entity.PermissionRecord{}, nil | |
| 83 | + | } | |
| 80 | 84 | ||
| 81 | 85 | records := make([]*entity.PermissionRecord, len(rows)) | |
| 82 | 86 | for i, row := range rows { | |
| @@ -73,10 +73,14 @@ func (r *permissionRepo) GetAll(ctx context.Context, origin string) ([]*entity.P | |||||
| 73 | log := logging.FromContext(ctx) | 73 | log := logging.FromContext(ctx) | ||
| 74 | log.Debug().Str("origin", origin).Msg("getting all permissions for origin") | 74 | log.Debug().Str("origin", origin).Msg("getting all permissions for origin") | ||
| 75 | 75 | ||||
| 76 | + | // sqlc returns an empty slice (not sql.ErrNoRows) when nothing matches. | |||
| 76 | rows, err := r.queries.ListPermissionsByOrigin(ctx, origin) | 77 | rows, err := r.queries.ListPermissionsByOrigin(ctx, origin) | ||
| 77 | if err != nil { | 78 | if err != nil { | ||
| 78 | return nil, err | 79 | return nil, err | ||
| 79 | } | 80 | } | ||
| 81 | + | if len(rows) == 0 { | |||
| 82 | + | return []*entity.PermissionRecord{}, nil | |||
| 83 | + | } | |||
| 80 | 84 | ||||
| 81 | records := make([]*entity.PermissionRecord, len(rows)) | 85 | records := make([]*entity.PermissionRecord, len(rows)) | ||
| 82 | for i, row := range rows { | 86 | for i, row := range rows { | ||
M internal/infrastructure/webkit/gobject_type_helpers.go +16 -5
| @@ -1,8 +1,10 @@ | |||
| 1 | 1 | package webkit | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "sync" | |
| 5 | 6 | ||
| 7 | + | "github.com/bnema/dumber/internal/logging" | |
| 6 | 8 | "github.com/jwijenbergh/purego" | |
| 7 | 9 | "github.com/jwijenbergh/puregotk/pkg/core" | |
| 8 | 10 | gtypes "github.com/jwijenbergh/puregotk/v4/gobject/types" | |
| @@ -14,39 +16,48 @@ var ( | |||
| 14 | 16 | typeNameFromInstance func(uintptr) string | |
| 15 | 17 | ) | |
| 16 | 18 | ||
| 17 | - | func gobjectTypeCheckInstanceIsAByPtr(instancePtr uintptr, requestType gtypes.GType) bool { | |
| 19 | + | func gobjectTypeCheckInstanceIsAByPtr(ctx context.Context, instancePtr uintptr, requestType gtypes.GType) bool { | |
| 18 | 20 | if instancePtr == 0 { | |
| 19 | 21 | return false | |
| 20 | 22 | } | |
| 21 | - | loadGObjectTypeFns() | |
| 23 | + | if ctx == nil { | |
| 24 | + | ctx = context.Background() | |
| 25 | + | } | |
| 26 | + | loadGObjectTypeFns(ctx) | |
| 22 | 27 | if typeCheckInstanceIsA == nil { | |
| 23 | 28 | return false | |
| 24 | 29 | } | |
| 25 | 30 | return typeCheckInstanceIsA(instancePtr, requestType) | |
| 26 | 31 | } | |
| 27 | 32 | ||
| 28 | - | func permissionRequestTypeName(instancePtr uintptr) string { | |
| 33 | + | func permissionRequestTypeName(ctx context.Context, instancePtr uintptr) string { | |
| 29 | 34 | if instancePtr == 0 { | |
| 30 | 35 | return "" | |
| 31 | 36 | } | |
| 32 | - | loadGObjectTypeFns() | |
| 37 | + | if ctx == nil { | |
| 38 | + | ctx = context.Background() | |
| 39 | + | } | |
| 40 | + | loadGObjectTypeFns(ctx) | |
| 33 | 41 | if typeNameFromInstance == nil { | |
| 34 | 42 | return "" | |
| 35 | 43 | } | |
| 36 | 44 | return typeNameFromInstance(instancePtr) | |
| 37 | 45 | } | |
| 38 | 46 | ||
| 39 | - | func loadGObjectTypeFns() { | |
| 47 | + | func loadGObjectTypeFns(ctx context.Context) { | |
| 40 | 48 | loadGObjectTypeFnsOnce.Do(func() { | |
| 49 | + | log := logging.FromContext(ctx) | |
| 41 | 50 | libs := make([]uintptr, 0, 2) | |
| 42 | 51 | for _, libPath := range core.GetPaths("GOBJECT") { | |
| 43 | 52 | lib, err := purego.Dlopen(libPath, purego.RTLD_NOW|purego.RTLD_GLOBAL) | |
| 44 | 53 | if err != nil { | |
| 54 | + | log.Debug().Str("path", libPath).Err(err).Msg("failed to load GObject library") | |
| 45 | 55 | continue | |
| 46 | 56 | } | |
| 47 | 57 | libs = append(libs, lib) | |
| 48 | 58 | } | |
| 49 | 59 | if len(libs) == 0 { | |
| 60 | + | log.Debug().Msg("no GObject libraries loaded; permission type helpers unavailable") | |
| 50 | 61 | return | |
| 51 | 62 | } | |
| 52 | 63 | ||
| @@ -1,8 +1,10 @@ | |||||
| 1 | package webkit | 1 | package webkit | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "sync" | 5 | "sync" | ||
| 5 | 6 | ||||
| 7 | + | "github.com/bnema/dumber/internal/logging" | |||
| 6 | "github.com/jwijenbergh/purego" | 8 | "github.com/jwijenbergh/purego" | ||
| 7 | "github.com/jwijenbergh/puregotk/pkg/core" | 9 | "github.com/jwijenbergh/puregotk/pkg/core" | ||
| 8 | gtypes "github.com/jwijenbergh/puregotk/v4/gobject/types" | 10 | gtypes "github.com/jwijenbergh/puregotk/v4/gobject/types" | ||
| @@ -14,39 +16,48 @@ var ( | |||||
| 14 | typeNameFromInstance func(uintptr) string | 16 | typeNameFromInstance func(uintptr) string | ||
| 15 | ) | 17 | ) | ||
| 16 | 18 | ||||
| 17 | - | func gobjectTypeCheckInstanceIsAByPtr(instancePtr uintptr, requestType gtypes.GType) bool { | 19 | + | func gobjectTypeCheckInstanceIsAByPtr(ctx context.Context, instancePtr uintptr, requestType gtypes.GType) bool { |
| 18 | if instancePtr == 0 { | 20 | if instancePtr == 0 { | ||
| 19 | return false | 21 | return false | ||
| 20 | } | 22 | } | ||
| 21 | - | loadGObjectTypeFns() | 23 | + | if ctx == nil { |
| 24 | + | ctx = context.Background() | |||
| 25 | + | } | |||
| 26 | + | loadGObjectTypeFns(ctx) | |||
| 22 | if typeCheckInstanceIsA == nil { | 27 | if typeCheckInstanceIsA == nil { | ||
| 23 | return false | 28 | return false | ||
| 24 | } | 29 | } | ||
| 25 | return typeCheckInstanceIsA(instancePtr, requestType) | 30 | return typeCheckInstanceIsA(instancePtr, requestType) | ||
| 26 | } | 31 | } | ||
| 27 | 32 | ||||
| 28 | - | func permissionRequestTypeName(instancePtr uintptr) string { | 33 | + | func permissionRequestTypeName(ctx context.Context, instancePtr uintptr) string { |
| 29 | if instancePtr == 0 { | 34 | if instancePtr == 0 { | ||
| 30 | return "" | 35 | return "" | ||
| 31 | } | 36 | } | ||
| 32 | - | loadGObjectTypeFns() | 37 | + | if ctx == nil { |
| 38 | + | ctx = context.Background() | |||
| 39 | + | } | |||
| 40 | + | loadGObjectTypeFns(ctx) | |||
| 33 | if typeNameFromInstance == nil { | 41 | if typeNameFromInstance == nil { | ||
| 34 | return "" | 42 | return "" | ||
| 35 | } | 43 | } | ||
| 36 | return typeNameFromInstance(instancePtr) | 44 | return typeNameFromInstance(instancePtr) | ||
| 37 | } | 45 | } | ||
| 38 | 46 | ||||
| 39 | - | func loadGObjectTypeFns() { | 47 | + | func loadGObjectTypeFns(ctx context.Context) { |
| 40 | loadGObjectTypeFnsOnce.Do(func() { | 48 | loadGObjectTypeFnsOnce.Do(func() { | ||
| 49 | + | log := logging.FromContext(ctx) | |||
| 41 | libs := make([]uintptr, 0, 2) | 50 | libs := make([]uintptr, 0, 2) | ||
| 42 | for _, libPath := range core.GetPaths("GOBJECT") { | 51 | for _, libPath := range core.GetPaths("GOBJECT") { | ||
| 43 | lib, err := purego.Dlopen(libPath, purego.RTLD_NOW|purego.RTLD_GLOBAL) | 52 | lib, err := purego.Dlopen(libPath, purego.RTLD_NOW|purego.RTLD_GLOBAL) | ||
| 44 | if err != nil { | 53 | if err != nil { | ||
| 54 | + | log.Debug().Str("path", libPath).Err(err).Msg("failed to load GObject library") | |||
| 45 | continue | 55 | continue | ||
| 46 | } | 56 | } | ||
| 47 | libs = append(libs, lib) | 57 | libs = append(libs, lib) | ||
| 48 | } | 58 | } | ||
| 49 | if len(libs) == 0 { | 59 | if len(libs) == 0 { | ||
| 60 | + | log.Debug().Msg("no GObject libraries loaded; permission type helpers unavailable") | |||
| 50 | return | 61 | return | ||
| 51 | } | 62 | } | ||
| 52 | 63 | ||||
M internal/infrastructure/webkit/injector_webrtc_compat_test.go +3 -3
| @@ -10,7 +10,7 @@ func TestBuildWebRTCCompatScript_AddsPrefixedAliases(t *testing.T) { | |||
| 10 | 10 | script := buildWebRTCCompatScript() | |
| 11 | 11 | ||
| 12 | 12 | assert.Contains(t, script, "window.webkitRTCPeerConnection") | |
| 13 | - | assert.Contains(t, script, "window.RTCPeerConnection = window.webkitRTCPeerConnection") | |
| 14 | - | assert.Contains(t, script, "window.RTCSessionDescription = window.webkitRTCSessionDescription") | |
| 15 | - | assert.Contains(t, script, "window.RTCIceCandidate = window.webkitRTCIceCandidate") | |
| 13 | + | assert.Regexp(t, `window\.RTCPeerConnection\s*=\s*window\.webkitRTCPeerConnection`, script) | |
| 14 | + | assert.Regexp(t, `window\.RTCSessionDescription\s*=\s*window\.webkitRTCSessionDescription`, script) | |
| 15 | + | assert.Regexp(t, `window\.RTCIceCandidate\s*=\s*window\.webkitRTCIceCandidate`, script) | |
| 16 | 16 | } | |
| @@ -10,7 +10,7 @@ func TestBuildWebRTCCompatScript_AddsPrefixedAliases(t *testing.T) { | |||||
| 10 | script := buildWebRTCCompatScript() | 10 | script := buildWebRTCCompatScript() | ||
| 11 | 11 | ||||
| 12 | assert.Contains(t, script, "window.webkitRTCPeerConnection") | 12 | assert.Contains(t, script, "window.webkitRTCPeerConnection") | ||
| 13 | - | assert.Contains(t, script, "window.RTCPeerConnection = window.webkitRTCPeerConnection") | 13 | + | assert.Regexp(t, `window\.RTCPeerConnection\s*=\s*window\.webkitRTCPeerConnection`, script) |
| 14 | - | assert.Contains(t, script, "window.RTCSessionDescription = window.webkitRTCSessionDescription") | 14 | + | assert.Regexp(t, `window\.RTCSessionDescription\s*=\s*window\.webkitRTCSessionDescription`, script) |
| 15 | - | assert.Contains(t, script, "window.RTCIceCandidate = window.webkitRTCIceCandidate") | 15 | + | assert.Regexp(t, `window\.RTCIceCandidate\s*=\s*window\.webkitRTCIceCandidate`, script) |
| 16 | } | 16 | } | ||
M internal/infrastructure/webkit/scheme_handler.go +2 -0
| @@ -562,6 +562,8 @@ func resolveAssetPath(u *url.URL) (string, bool) { | |||
| 562 | 562 | switch u.Opaque { | |
| 563 | 563 | case HomePath: | |
| 564 | 564 | return IndexHTML, true | |
| 565 | + | case ConfigPath: | |
| 566 | + | return "config.html", true | |
| 565 | 567 | case ErrorPath: | |
| 566 | 568 | return "error.html", true | |
| 567 | 569 | case WebRTCPath: | |
| @@ -562,6 +562,8 @@ func resolveAssetPath(u *url.URL) (string, bool) { | |||||
| 562 | switch u.Opaque { | 562 | switch u.Opaque { | ||
| 563 | case HomePath: | 563 | case HomePath: | ||
| 564 | return IndexHTML, true | 564 | return IndexHTML, true | ||
| 565 | + | case ConfigPath: | |||
| 566 | + | return "config.html", true | |||
| 565 | case ErrorPath: | 567 | case ErrorPath: | ||
| 566 | return "error.html", true | 568 | return "error.html", true | ||
| 567 | case WebRTCPath: | 569 | case WebRTCPath: | ||
M internal/infrastructure/webkit/scheme_handler_test.go +13 -0
| @@ -115,3 +115,16 @@ func TestHandleAsset_WebRTCOpaqueFormServesInternalWebRTCTester(t *testing.T) { | |||
| 115 | 115 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |
| 116 | 116 | assert.Contains(t, string(resp.Data), "webrtc.min.js") | |
| 117 | 117 | } | |
| 118 | + | ||
| 119 | + | func TestHandleAsset_ConfigOpaqueFormServesConfigPage(t *testing.T) { | |
| 120 | + | h := NewDumbSchemeHandler(context.Background()) | |
| 121 | + | h.SetAssets(assets.WebUIAssets) | |
| 122 | + | ||
| 123 | + | u, err := url.Parse("dumb:config") | |
| 124 | + | require.NoError(t, err) | |
| 125 | + | ||
| 126 | + | resp := h.handleAsset(u) | |
| 127 | + | require.NotNil(t, resp) | |
| 128 | + | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |
| 129 | + | assert.Contains(t, string(resp.Data), "config") | |
| 130 | + | } | |
| @@ -115,3 +115,16 @@ func TestHandleAsset_WebRTCOpaqueFormServesInternalWebRTCTester(t *testing.T) { | |||||
| 115 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | 115 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | ||
| 116 | assert.Contains(t, string(resp.Data), "webrtc.min.js") | 116 | assert.Contains(t, string(resp.Data), "webrtc.min.js") | ||
| 117 | } | 117 | } | ||
| 118 | + | ||||
| 119 | + | func TestHandleAsset_ConfigOpaqueFormServesConfigPage(t *testing.T) { | |||
| 120 | + | h := NewDumbSchemeHandler(context.Background()) | |||
| 121 | + | h.SetAssets(assets.WebUIAssets) | |||
| 122 | + | ||||
| 123 | + | u, err := url.Parse("dumb:config") | |||
| 124 | + | require.NoError(t, err) | |||
| 125 | + | ||||
| 126 | + | resp := h.handleAsset(u) | |||
| 127 | + | require.NotNil(t, resp) | |||
| 128 | + | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |||
| 129 | + | assert.Contains(t, string(resp.Data), "config") | |||
| 130 | + | } | |||
M internal/infrastructure/webkit/settings.go +1 -1
| @@ -122,7 +122,6 @@ func applyMediaSettings(settings *webkit.Settings, cfg *config.Config, log *zero | |||
| 122 | 122 | settings.SetEnableMedia(true) | |
| 123 | 123 | settings.SetEnableMediasource(true) | |
| 124 | 124 | settings.SetEnableMediaCapabilities(true) | |
| 125 | - | settings.SetEnableMediaStream(true) | |
| 126 | 125 | settings.SetEnableEncryptedMedia(true) | |
| 127 | 126 | settings.SetMediaPlaybackRequiresUserGesture(true) | |
| 128 | 127 | settings.SetMediaPlaybackAllowsInline(true) | |
| @@ -161,6 +160,7 @@ func applyWebRTCSettings(settings *webkit.Settings) { | |||
| 161 | 160 | settings.SetPropertyEnableMediaStream(true) | |
| 162 | 161 | settings.SetPropertyEnableWebrtc(true) | |
| 163 | 162 | ||
| 163 | + | settings.SetEnableMediaStream(true) | |
| 164 | 164 | settings.SetEnableWebrtc(true) | |
| 165 | 165 | } | |
| 166 | 166 | ||
| @@ -122,7 +122,6 @@ func applyMediaSettings(settings *webkit.Settings, cfg *config.Config, log *zero | |||||
| 122 | settings.SetEnableMedia(true) | 122 | settings.SetEnableMedia(true) | ||
| 123 | settings.SetEnableMediasource(true) | 123 | settings.SetEnableMediasource(true) | ||
| 124 | settings.SetEnableMediaCapabilities(true) | 124 | settings.SetEnableMediaCapabilities(true) | ||
| 125 | - | settings.SetEnableMediaStream(true) | |||
| 126 | settings.SetEnableEncryptedMedia(true) | 125 | settings.SetEnableEncryptedMedia(true) | ||
| 127 | settings.SetMediaPlaybackRequiresUserGesture(true) | 126 | settings.SetMediaPlaybackRequiresUserGesture(true) | ||
| 128 | settings.SetMediaPlaybackAllowsInline(true) | 127 | settings.SetMediaPlaybackAllowsInline(true) | ||
| @@ -161,6 +160,7 @@ func applyWebRTCSettings(settings *webkit.Settings) { | |||||
| 161 | settings.SetPropertyEnableMediaStream(true) | 160 | settings.SetPropertyEnableMediaStream(true) | ||
| 162 | settings.SetPropertyEnableWebrtc(true) | 161 | settings.SetPropertyEnableWebrtc(true) | ||
| 163 | 162 | ||||
| 163 | + | settings.SetEnableMediaStream(true) | |||
| 164 | settings.SetEnableWebrtc(true) | 164 | settings.SetEnableWebrtc(true) | ||
| 165 | } | 165 | } | ||
| 166 | 166 | ||||
M internal/infrastructure/webkit/webview.go +25 -13
| @@ -975,6 +975,8 @@ func (wv *WebView) connectWebProcessTerminatedSignal() { | |||
| 975 | 975 | // This is emitted when a site calls getUserMedia() or getDisplayMedia(). | |
| 976 | 976 | func (wv *WebView) connectPermissionRequestSignal() { | |
| 977 | 977 | permissionCb := func(_ webkit.WebView, requestPtr uintptr) bool { | |
| 978 | + | ctx := logging.WithContext(context.Background(), wv.logger) | |
| 979 | + | ||
| 978 | 980 | if wv.OnPermissionRequest == nil { | |
| 979 | 981 | return false // Not handled, WebKit will deny by default | |
| 980 | 982 | } | |
| @@ -992,7 +994,7 @@ func (wv *WebView) connectPermissionRequestSignal() { | |||
| 992 | 994 | } | |
| 993 | 995 | ||
| 994 | 996 | // Determine permission types from the request | |
| 995 | - | permTypes := wv.determinePermissionTypes(requestPtr) | |
| 997 | + | permTypes := wv.determinePermissionTypes(ctx, requestPtr) | |
| 996 | 998 | if len(permTypes) == 0 { | |
| 997 | 999 | wv.logger.Warn().Msg("permission request with unknown type, denying") | |
| 998 | 1000 | return false | |
| @@ -1042,8 +1044,8 @@ func (wv *WebView) connectPermissionRequestSignal() { | |||
| 1042 | 1044 | // We use GObject property accessors for audio/video request flags. Display detection | |
| 1043 | 1045 | // uses the dedicated WebKit API because this WebKit build does not expose an | |
| 1044 | 1046 | // "is-for-display-device" GObject property. | |
| 1045 | - | func (wv *WebView) determinePermissionTypes(requestPtr uintptr) []string { | |
| 1046 | - | requestKind := detectPermissionRequestKind(requestPtr) | |
| 1047 | + | func (wv *WebView) determinePermissionTypes(ctx context.Context, requestPtr uintptr) []string { | |
| 1048 | + | requestKind := detectPermissionRequestKind(ctx, requestPtr) | |
| 1047 | 1049 | switch requestKind { | |
| 1048 | 1050 | case permissionRequestKindUserMedia: | |
| 1049 | 1051 | userMediaReq := webkit.UserMediaPermissionRequestNewFromInternalPtr(requestPtr) | |
| @@ -1063,12 +1065,12 @@ func (wv *WebView) determinePermissionTypes(requestPtr uintptr) []string { | |||
| 1063 | 1065 | Bool("is_display", isDisplay). | |
| 1064 | 1066 | Msg("permission request type detection") | |
| 1065 | 1067 | ||
| 1066 | - | return classifyPermissionRequestTypes(requestKind, isAudio, isVideo, isDisplay) | |
| 1068 | + | return classifyPermissionRequestTypes(ctx, requestKind, isAudio, isVideo, isDisplay) | |
| 1067 | 1069 | case permissionRequestKindDeviceInfo: | |
| 1068 | - | return classifyPermissionRequestTypes(requestKind, false, false, false) | |
| 1070 | + | return classifyPermissionRequestTypes(ctx, requestKind, false, false, false) | |
| 1069 | 1071 | default: | |
| 1070 | 1072 | if requestPtr != 0 { | |
| 1071 | - | typeName := permissionRequestTypeName(requestPtr) | |
| 1073 | + | typeName := permissionRequestTypeName(ctx, requestPtr) | |
| 1072 | 1074 | if typeName != "" { | |
| 1073 | 1075 | wv.logger.Warn().Str("request_type", typeName).Msg("unknown permission request type") | |
| 1074 | 1076 | } else { | |
| @@ -1089,27 +1091,28 @@ const ( | |||
| 1089 | 1091 | permissionRequestKindDeviceInfo | |
| 1090 | 1092 | ) | |
| 1091 | 1093 | ||
| 1092 | - | func detectPermissionRequestKind(requestPtr uintptr) permissionRequestKind { | |
| 1093 | - | if isPermissionRequestType(requestPtr, webkit.UserMediaPermissionRequestGLibType()) { | |
| 1094 | + | func detectPermissionRequestKind(ctx context.Context, requestPtr uintptr) permissionRequestKind { | |
| 1095 | + | if isPermissionRequestType(ctx, requestPtr, webkit.UserMediaPermissionRequestGLibType()) { | |
| 1094 | 1096 | return permissionRequestKindUserMedia | |
| 1095 | 1097 | } | |
| 1096 | - | if isPermissionRequestType(requestPtr, webkit.DeviceInfoPermissionRequestGLibType()) { | |
| 1098 | + | if isPermissionRequestType(ctx, requestPtr, webkit.DeviceInfoPermissionRequestGLibType()) { | |
| 1097 | 1099 | return permissionRequestKindDeviceInfo | |
| 1098 | 1100 | } | |
| 1099 | 1101 | return permissionRequestKindUnknown | |
| 1100 | 1102 | } | |
| 1101 | 1103 | ||
| 1102 | - | func isPermissionRequestType(requestPtr uintptr, requestType gtypes.GType) bool { | |
| 1103 | - | return gobjectTypeCheckInstanceIsAByPtr(requestPtr, requestType) | |
| 1104 | + | func isPermissionRequestType(ctx context.Context, requestPtr uintptr, requestType gtypes.GType) bool { | |
| 1105 | + | return gobjectTypeCheckInstanceIsAByPtr(ctx, requestPtr, requestType) | |
| 1104 | 1106 | } | |
| 1105 | 1107 | ||
| 1106 | 1108 | func classifyPermissionRequestTypes( | |
| 1109 | + | ctx context.Context, | |
| 1107 | 1110 | kind permissionRequestKind, | |
| 1108 | 1111 | isAudio, isVideo, isDisplay bool, | |
| 1109 | 1112 | ) []string { | |
| 1110 | 1113 | switch kind { | |
| 1111 | 1114 | case permissionRequestKindUserMedia: | |
| 1112 | - | return classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay) | |
| 1115 | + | return classifyUserMediaPermissionTypes(ctx, isAudio, isVideo, isDisplay) | |
| 1113 | 1116 | case permissionRequestKindDeviceInfo: | |
| 1114 | 1117 | return []string{"device_info"} | |
| 1115 | 1118 | default: | |
| @@ -1117,7 +1120,7 @@ func classifyPermissionRequestTypes( | |||
| 1117 | 1120 | } | |
| 1118 | 1121 | } | |
| 1119 | 1122 | ||
| 1120 | - | func classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay bool) []string { | |
| 1123 | + | func classifyUserMediaPermissionTypes(ctx context.Context, isAudio, isVideo, isDisplay bool) []string { | |
| 1121 | 1124 | types := make([]string, 0, 2) | |
| 1122 | 1125 | ||
| 1123 | 1126 | if isAudio { | |
| @@ -1133,6 +1136,11 @@ func classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay bool) []string | |||
| 1133 | 1136 | if len(types) == 0 { | |
| 1134 | 1137 | // On some Wayland/WebKit paths, getDisplayMedia() can arrive with all | |
| 1135 | 1138 | // flags false even though the request is a screen-capture request. | |
| 1139 | + | logging.FromContext(ctx).Debug(). | |
| 1140 | + | Bool("is_audio", isAudio). | |
| 1141 | + | Bool("is_video", isVideo). | |
| 1142 | + | Bool("is_display", isDisplay). | |
| 1143 | + | Msg("user media request had no flags; using display fallback") | |
| 1136 | 1144 | return []string{"display"} | |
| 1137 | 1145 | } | |
| 1138 | 1146 | ||
| @@ -1156,6 +1164,8 @@ func (wv *WebView) allowPermissionRequest(requestPtr uintptr) { | |||
| 1156 | 1164 | wv.logger.Debug().Msg("permission request allowed") | |
| 1157 | 1165 | return | |
| 1158 | 1166 | } | |
| 1167 | + | ||
| 1168 | + | wv.logger.Warn().Uint64("request_ptr", uint64(requestPtr)).Msg("permission request: unknown type, cannot allow") | |
| 1159 | 1169 | } | |
| 1160 | 1170 | ||
| 1161 | 1171 | // denyPermissionRequest calls Deny() on the WebKit permission request. | |
| @@ -1175,6 +1185,8 @@ func (wv *WebView) denyPermissionRequest(requestPtr uintptr) { | |||
| 1175 | 1185 | wv.logger.Debug().Msg("permission request denied") | |
| 1176 | 1186 | return | |
| 1177 | 1187 | } | |
| 1188 | + | ||
| 1189 | + | wv.logger.Warn().Uint64("request_ptr", uint64(requestPtr)).Msg("permission request: unknown type, cannot deny") | |
| 1178 | 1190 | } | |
| 1179 | 1191 | ||
| 1180 | 1192 | // ID returns the unique identifier for this WebView. | |
| @@ -975,6 +975,8 @@ func (wv *WebView) connectWebProcessTerminatedSignal() { | |||||
| 975 | // This is emitted when a site calls getUserMedia() or getDisplayMedia(). | 975 | // This is emitted when a site calls getUserMedia() or getDisplayMedia(). | ||
| 976 | func (wv *WebView) connectPermissionRequestSignal() { | 976 | func (wv *WebView) connectPermissionRequestSignal() { | ||
| 977 | permissionCb := func(_ webkit.WebView, requestPtr uintptr) bool { | 977 | permissionCb := func(_ webkit.WebView, requestPtr uintptr) bool { | ||
| 978 | + | ctx := logging.WithContext(context.Background(), wv.logger) | |||
| 979 | + | ||||
| 978 | if wv.OnPermissionRequest == nil { | 980 | if wv.OnPermissionRequest == nil { | ||
| 979 | return false // Not handled, WebKit will deny by default | 981 | return false // Not handled, WebKit will deny by default | ||
| 980 | } | 982 | } | ||
| @@ -992,7 +994,7 @@ func (wv *WebView) connectPermissionRequestSignal() { | |||||
| 992 | } | 994 | } | ||
| 993 | 995 | ||||
| 994 | // Determine permission types from the request | 996 | // Determine permission types from the request | ||
| 995 | - | permTypes := wv.determinePermissionTypes(requestPtr) | 997 | + | permTypes := wv.determinePermissionTypes(ctx, requestPtr) |
| 996 | if len(permTypes) == 0 { | 998 | if len(permTypes) == 0 { | ||
| 997 | wv.logger.Warn().Msg("permission request with unknown type, denying") | 999 | wv.logger.Warn().Msg("permission request with unknown type, denying") | ||
| 998 | return false | 1000 | return false | ||
| @@ -1042,8 +1044,8 @@ func (wv *WebView) connectPermissionRequestSignal() { | |||||
| 1042 | // We use GObject property accessors for audio/video request flags. Display detection | 1044 | // We use GObject property accessors for audio/video request flags. Display detection | ||
| 1043 | // uses the dedicated WebKit API because this WebKit build does not expose an | 1045 | // uses the dedicated WebKit API because this WebKit build does not expose an | ||
| 1044 | // "is-for-display-device" GObject property. | 1046 | // "is-for-display-device" GObject property. | ||
| 1045 | - | func (wv *WebView) determinePermissionTypes(requestPtr uintptr) []string { | 1047 | + | func (wv *WebView) determinePermissionTypes(ctx context.Context, requestPtr uintptr) []string { |
| 1046 | - | requestKind := detectPermissionRequestKind(requestPtr) | 1048 | + | requestKind := detectPermissionRequestKind(ctx, requestPtr) |
| 1047 | switch requestKind { | 1049 | switch requestKind { | ||
| 1048 | case permissionRequestKindUserMedia: | 1050 | case permissionRequestKindUserMedia: | ||
| 1049 | userMediaReq := webkit.UserMediaPermissionRequestNewFromInternalPtr(requestPtr) | 1051 | userMediaReq := webkit.UserMediaPermissionRequestNewFromInternalPtr(requestPtr) | ||
| @@ -1063,12 +1065,12 @@ func (wv *WebView) determinePermissionTypes(requestPtr uintptr) []string { | |||||
| 1063 | Bool("is_display", isDisplay). | 1065 | Bool("is_display", isDisplay). | ||
| 1064 | Msg("permission request type detection") | 1066 | Msg("permission request type detection") | ||
| 1065 | 1067 | ||||
| 1066 | - | return classifyPermissionRequestTypes(requestKind, isAudio, isVideo, isDisplay) | 1068 | + | return classifyPermissionRequestTypes(ctx, requestKind, isAudio, isVideo, isDisplay) |
| 1067 | case permissionRequestKindDeviceInfo: | 1069 | case permissionRequestKindDeviceInfo: | ||
| 1068 | - | return classifyPermissionRequestTypes(requestKind, false, false, false) | 1070 | + | return classifyPermissionRequestTypes(ctx, requestKind, false, false, false) |
| 1069 | default: | 1071 | default: | ||
| 1070 | if requestPtr != 0 { | 1072 | if requestPtr != 0 { | ||
| 1071 | - | typeName := permissionRequestTypeName(requestPtr) | 1073 | + | typeName := permissionRequestTypeName(ctx, requestPtr) |
| 1072 | if typeName != "" { | 1074 | if typeName != "" { | ||
| 1073 | wv.logger.Warn().Str("request_type", typeName).Msg("unknown permission request type") | 1075 | wv.logger.Warn().Str("request_type", typeName).Msg("unknown permission request type") | ||
| 1074 | } else { | 1076 | } else { | ||
| @@ -1089,27 +1091,28 @@ const ( | |||||
| 1089 | permissionRequestKindDeviceInfo | 1091 | permissionRequestKindDeviceInfo | ||
| 1090 | ) | 1092 | ) | ||
| 1091 | 1093 | ||||
| 1092 | - | func detectPermissionRequestKind(requestPtr uintptr) permissionRequestKind { | 1094 | + | func detectPermissionRequestKind(ctx context.Context, requestPtr uintptr) permissionRequestKind { |
| 1093 | - | if isPermissionRequestType(requestPtr, webkit.UserMediaPermissionRequestGLibType()) { | 1095 | + | if isPermissionRequestType(ctx, requestPtr, webkit.UserMediaPermissionRequestGLibType()) { |
| 1094 | return permissionRequestKindUserMedia | 1096 | return permissionRequestKindUserMedia | ||
| 1095 | } | 1097 | } | ||
| 1096 | - | if isPermissionRequestType(requestPtr, webkit.DeviceInfoPermissionRequestGLibType()) { | 1098 | + | if isPermissionRequestType(ctx, requestPtr, webkit.DeviceInfoPermissionRequestGLibType()) { |
| 1097 | return permissionRequestKindDeviceInfo | 1099 | return permissionRequestKindDeviceInfo | ||
| 1098 | } | 1100 | } | ||
| 1099 | return permissionRequestKindUnknown | 1101 | return permissionRequestKindUnknown | ||
| 1100 | } | 1102 | } | ||
| 1101 | 1103 | ||||
| 1102 | - | func isPermissionRequestType(requestPtr uintptr, requestType gtypes.GType) bool { | 1104 | + | func isPermissionRequestType(ctx context.Context, requestPtr uintptr, requestType gtypes.GType) bool { |
| 1103 | - | return gobjectTypeCheckInstanceIsAByPtr(requestPtr, requestType) | 1105 | + | return gobjectTypeCheckInstanceIsAByPtr(ctx, requestPtr, requestType) |
| 1104 | } | 1106 | } | ||
| 1105 | 1107 | ||||
| 1106 | func classifyPermissionRequestTypes( | 1108 | func classifyPermissionRequestTypes( | ||
| 1109 | + | ctx context.Context, | |||
| 1107 | kind permissionRequestKind, | 1110 | kind permissionRequestKind, | ||
| 1108 | isAudio, isVideo, isDisplay bool, | 1111 | isAudio, isVideo, isDisplay bool, | ||
| 1109 | ) []string { | 1112 | ) []string { | ||
| 1110 | switch kind { | 1113 | switch kind { | ||
| 1111 | case permissionRequestKindUserMedia: | 1114 | case permissionRequestKindUserMedia: | ||
| 1112 | - | return classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay) | 1115 | + | return classifyUserMediaPermissionTypes(ctx, isAudio, isVideo, isDisplay) |
| 1113 | case permissionRequestKindDeviceInfo: | 1116 | case permissionRequestKindDeviceInfo: | ||
| 1114 | return []string{"device_info"} | 1117 | return []string{"device_info"} | ||
| 1115 | default: | 1118 | default: | ||
| @@ -1117,7 +1120,7 @@ func classifyPermissionRequestTypes( | |||||
| 1117 | } | 1120 | } | ||
| 1118 | } | 1121 | } | ||
| 1119 | 1122 | ||||
| 1120 | - | func classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay bool) []string { | 1123 | + | func classifyUserMediaPermissionTypes(ctx context.Context, isAudio, isVideo, isDisplay bool) []string { |
| 1121 | types := make([]string, 0, 2) | 1124 | types := make([]string, 0, 2) | ||
| 1122 | 1125 | ||||
| 1123 | if isAudio { | 1126 | if isAudio { | ||
| @@ -1133,6 +1136,11 @@ func classifyUserMediaPermissionTypes(isAudio, isVideo, isDisplay bool) []string | |||||
| 1133 | if len(types) == 0 { | 1136 | if len(types) == 0 { | ||
| 1134 | // On some Wayland/WebKit paths, getDisplayMedia() can arrive with all | 1137 | // On some Wayland/WebKit paths, getDisplayMedia() can arrive with all | ||
| 1135 | // flags false even though the request is a screen-capture request. | 1138 | // flags false even though the request is a screen-capture request. | ||
| 1139 | + | logging.FromContext(ctx).Debug(). | |||
| 1140 | + | Bool("is_audio", isAudio). | |||
| 1141 | + | Bool("is_video", isVideo). | |||
| 1142 | + | Bool("is_display", isDisplay). | |||
| 1143 | + | Msg("user media request had no flags; using display fallback") | |||
| 1136 | return []string{"display"} | 1144 | return []string{"display"} | ||
| 1137 | } | 1145 | } | ||
| 1138 | 1146 | ||||
| @@ -1156,6 +1164,8 @@ func (wv *WebView) allowPermissionRequest(requestPtr uintptr) { | |||||
| 1156 | wv.logger.Debug().Msg("permission request allowed") | 1164 | wv.logger.Debug().Msg("permission request allowed") | ||
| 1157 | return | 1165 | return | ||
| 1158 | } | 1166 | } | ||
| 1167 | + | ||||
| 1168 | + | wv.logger.Warn().Uint64("request_ptr", uint64(requestPtr)).Msg("permission request: unknown type, cannot allow") | |||
| 1159 | } | 1169 | } | ||
| 1160 | 1170 | ||||
| 1161 | // denyPermissionRequest calls Deny() on the WebKit permission request. | 1171 | // denyPermissionRequest calls Deny() on the WebKit permission request. | ||
| @@ -1175,6 +1185,8 @@ func (wv *WebView) denyPermissionRequest(requestPtr uintptr) { | |||||
| 1175 | wv.logger.Debug().Msg("permission request denied") | 1185 | wv.logger.Debug().Msg("permission request denied") | ||
| 1176 | return | 1186 | return | ||
| 1177 | } | 1187 | } | ||
| 1188 | + | ||||
| 1189 | + | wv.logger.Warn().Uint64("request_ptr", uint64(requestPtr)).Msg("permission request: unknown type, cannot deny") | |||
| 1178 | } | 1190 | } | ||
| 1179 | 1191 | ||||
| 1180 | // ID returns the unique identifier for this WebView. | 1192 | // ID returns the unique identifier for this WebView. | ||
M internal/infrastructure/webkit/webview_permission_types_test.go +3 -2
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package webkit | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/assert" | |
| @@ -46,7 +47,7 @@ func TestClassifyUserMediaPermissionTypes(t *testing.T) { | |||
| 46 | 47 | ||
| 47 | 48 | for _, tt := range tests { | |
| 48 | 49 | t.Run(tt.name, func(t *testing.T) { | |
| 49 | - | assert.Equal(t, tt.expected, classifyUserMediaPermissionTypes(tt.isAudio, tt.isVideo, tt.isDisplay)) | |
| 50 | + | assert.Equal(t, tt.expected, classifyUserMediaPermissionTypes(context.Background(), tt.isAudio, tt.isVideo, tt.isDisplay)) | |
| 50 | 51 | }) | |
| 51 | 52 | } | |
| 52 | 53 | } | |
| @@ -90,7 +91,7 @@ func TestClassifyPermissionRequestTypes(t *testing.T) { | |||
| 90 | 91 | ||
| 91 | 92 | for _, tt := range tests { | |
| 92 | 93 | t.Run(tt.name, func(t *testing.T) { | |
| 93 | - | assert.Equal(t, tt.expected, classifyPermissionRequestTypes(tt.kind, tt.isAudio, tt.isVideo, tt.isDisplay)) | |
| 94 | + | assert.Equal(t, tt.expected, classifyPermissionRequestTypes(context.Background(), tt.kind, tt.isAudio, tt.isVideo, tt.isDisplay)) | |
| 94 | 95 | }) | |
| 95 | 96 | } | |
| 96 | 97 | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package webkit | 1 | package webkit | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| @@ -46,7 +47,7 @@ func TestClassifyUserMediaPermissionTypes(t *testing.T) { | |||||
| 46 | 47 | ||||
| 47 | for _, tt := range tests { | 48 | for _, tt := range tests { | ||
| 48 | t.Run(tt.name, func(t *testing.T) { | 49 | t.Run(tt.name, func(t *testing.T) { | ||
| 49 | - | assert.Equal(t, tt.expected, classifyUserMediaPermissionTypes(tt.isAudio, tt.isVideo, tt.isDisplay)) | 50 | + | assert.Equal(t, tt.expected, classifyUserMediaPermissionTypes(context.Background(), tt.isAudio, tt.isVideo, tt.isDisplay)) |
| 50 | }) | 51 | }) | ||
| 51 | } | 52 | } | ||
| 52 | } | 53 | } | ||
| @@ -90,7 +91,7 @@ func TestClassifyPermissionRequestTypes(t *testing.T) { | |||||
| 90 | 91 | ||||
| 91 | for _, tt := range tests { | 92 | for _, tt := range tests { | ||
| 92 | t.Run(tt.name, func(t *testing.T) { | 93 | t.Run(tt.name, func(t *testing.T) { | ||
| 93 | - | assert.Equal(t, tt.expected, classifyPermissionRequestTypes(tt.kind, tt.isAudio, tt.isVideo, tt.isDisplay)) | 94 | + | assert.Equal(t, tt.expected, classifyPermissionRequestTypes(context.Background(), tt.kind, tt.isAudio, tt.isVideo, tt.isDisplay)) |
| 94 | }) | 95 | }) | ||
| 95 | } | 96 | } | ||
| 96 | } | 97 | } | ||
M internal/ui/component/modal_sizing.go +1 -1
| @@ -88,7 +88,7 @@ var PermissionPopupSizeDefaults = ModalSizeConfig{ | |||
| 88 | 88 | MaxWidth: 400, | |
| 89 | 89 | TopMarginPct: 0.3, | |
| 90 | 90 | FallbackWidth: 400, | |
| 91 | - | FallbackHeight: 600, | |
| 91 | + | FallbackHeight: 300, | |
| 92 | 92 | } | |
| 93 | 93 | ||
| 94 | 94 | // EffectiveMaxRows returns the maximum number of visible rows that fit | |
| @@ -88,7 +88,7 @@ var PermissionPopupSizeDefaults = ModalSizeConfig{ | |||||
| 88 | MaxWidth: 400, | 88 | MaxWidth: 400, | ||
| 89 | TopMarginPct: 0.3, | 89 | TopMarginPct: 0.3, | ||
| 90 | FallbackWidth: 400, | 90 | FallbackWidth: 400, | ||
| 91 | - | FallbackHeight: 600, | 91 | + | FallbackHeight: 300, |
| 92 | } | 92 | } | ||
| 93 | 93 | ||||
| 94 | // EffectiveMaxRows returns the maximum number of visible rows that fit | 94 | // EffectiveMaxRows returns the maximum number of visible rows that fit | ||
M internal/ui/component/modal_sizing_test.go +9 -0
| @@ -137,6 +137,15 @@ func TestSessionManagerSizeDefaults_Values(t *testing.T) { | |||
| 137 | 137 | assert.LessOrEqual(t, SessionManagerSizeDefaults.TopMarginPct, 1.0) | |
| 138 | 138 | } | |
| 139 | 139 | ||
| 140 | + | func TestPermissionPopupSizeDefaults_Values(t *testing.T) { | |
| 141 | + | assert.Greater(t, PermissionPopupSizeDefaults.WidthPct, 0.0) | |
| 142 | + | assert.LessOrEqual(t, PermissionPopupSizeDefaults.WidthPct, 1.0) | |
| 143 | + | assert.Positive(t, PermissionPopupSizeDefaults.MaxWidth) | |
| 144 | + | assert.Greater(t, PermissionPopupSizeDefaults.TopMarginPct, 0.0) | |
| 145 | + | assert.LessOrEqual(t, PermissionPopupSizeDefaults.TopMarginPct, 1.0) | |
| 146 | + | assert.Equal(t, 300, PermissionPopupSizeDefaults.FallbackHeight) | |
| 147 | + | } | |
| 148 | + | ||
| 140 | 149 | func TestListDisplayDefaults_Values(t *testing.T) { | |
| 141 | 150 | assert.Positive(t, OmniboxListDefaults.MaxVisibleRows) | |
| 142 | 151 | assert.Positive(t, OmniboxListDefaults.MaxResults) | |
| @@ -137,6 +137,15 @@ func TestSessionManagerSizeDefaults_Values(t *testing.T) { | |||||
| 137 | assert.LessOrEqual(t, SessionManagerSizeDefaults.TopMarginPct, 1.0) | 137 | assert.LessOrEqual(t, SessionManagerSizeDefaults.TopMarginPct, 1.0) | ||
| 138 | } | 138 | } | ||
| 139 | 139 | ||||
| 140 | + | func TestPermissionPopupSizeDefaults_Values(t *testing.T) { | |||
| 141 | + | assert.Greater(t, PermissionPopupSizeDefaults.WidthPct, 0.0) | |||
| 142 | + | assert.LessOrEqual(t, PermissionPopupSizeDefaults.WidthPct, 1.0) | |||
| 143 | + | assert.Positive(t, PermissionPopupSizeDefaults.MaxWidth) | |||
| 144 | + | assert.Greater(t, PermissionPopupSizeDefaults.TopMarginPct, 0.0) | |||
| 145 | + | assert.LessOrEqual(t, PermissionPopupSizeDefaults.TopMarginPct, 1.0) | |||
| 146 | + | assert.Equal(t, 300, PermissionPopupSizeDefaults.FallbackHeight) | |||
| 147 | + | } | |||
| 148 | + | ||||
| 140 | func TestListDisplayDefaults_Values(t *testing.T) { | 149 | func TestListDisplayDefaults_Values(t *testing.T) { | ||
| 141 | assert.Positive(t, OmniboxListDefaults.MaxVisibleRows) | 150 | assert.Positive(t, OmniboxListDefaults.MaxVisibleRows) | ||
| 142 | assert.Positive(t, OmniboxListDefaults.MaxResults) | 151 | assert.Positive(t, OmniboxListDefaults.MaxResults) | ||
M internal/ui/component/webrtc_permission_indicator.go +2 -6
| @@ -298,12 +298,8 @@ func (w *WebRTCPermissionIndicator) Reset() { | |||
| 298 | 298 | w.mu.Lock() | |
| 299 | 299 | defer w.mu.Unlock() | |
| 300 | 300 | ||
| 301 | - | for k := range w.states { | |
| 302 | - | delete(w.states, k) | |
| 303 | - | } | |
| 304 | - | for k := range w.stored { | |
| 305 | - | delete(w.stored, k) | |
| 306 | - | } | |
| 301 | + | clear(w.states) | |
| 302 | + | clear(w.stored) | |
| 307 | 303 | w.currentOrigin = "" | |
| 308 | 304 | ||
| 309 | 305 | w.refreshLocked() | |
| @@ -298,12 +298,8 @@ func (w *WebRTCPermissionIndicator) Reset() { | |||||
| 298 | w.mu.Lock() | 298 | w.mu.Lock() | ||
| 299 | defer w.mu.Unlock() | 299 | defer w.mu.Unlock() | ||
| 300 | 300 | ||||
| 301 | - | for k := range w.states { | 301 | + | clear(w.states) |
| 302 | - | delete(w.states, k) | 302 | + | clear(w.stored) |
| 303 | - | } | |||
| 304 | - | for k := range w.stored { | |||
| 305 | - | delete(w.stored, k) | |||
| 306 | - | } | |||
| 307 | w.currentOrigin = "" | 303 | w.currentOrigin = "" | ||
| 308 | 304 | ||||
| 309 | w.refreshLocked() | 305 | w.refreshLocked() | ||
M internal/ui/component/webrtc_permission_indicator_state_test.go +9 -0
| @@ -41,6 +41,15 @@ func TestWebRTCSummaryState_IdleWhenEmpty(t *testing.T) { | |||
| 41 | 41 | assert.Equal(t, webrtcPermissionStateIdle, summarizeWebRTCPermissionState(map[entity.PermissionType]webrtcPermissionState{})) | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | + | func TestWebRTCSummaryState_IdleWhenAllIdle(t *testing.T) { | |
| 45 | + | states := map[entity.PermissionType]webrtcPermissionState{ | |
| 46 | + | entity.PermissionTypeMicrophone: webrtcPermissionStateIdle, | |
| 47 | + | entity.PermissionTypeCamera: webrtcPermissionStateIdle, | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | assert.Equal(t, webrtcPermissionStateIdle, summarizeWebRTCPermissionState(states)) | |
| 51 | + | } | |
| 52 | + | ||
| 44 | 53 | func TestWebRTCShouldShowIndicator(t *testing.T) { | |
| 45 | 54 | assert.False(t, shouldShowWebRTCPermissionIndicator(nil)) | |
| 46 | 55 | assert.False(t, shouldShowWebRTCPermissionIndicator(map[entity.PermissionType]webrtcPermissionState{ | |
| @@ -41,6 +41,15 @@ func TestWebRTCSummaryState_IdleWhenEmpty(t *testing.T) { | |||||
| 41 | assert.Equal(t, webrtcPermissionStateIdle, summarizeWebRTCPermissionState(map[entity.PermissionType]webrtcPermissionState{})) | 41 | assert.Equal(t, webrtcPermissionStateIdle, summarizeWebRTCPermissionState(map[entity.PermissionType]webrtcPermissionState{})) | ||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | + | func TestWebRTCSummaryState_IdleWhenAllIdle(t *testing.T) { | |||
| 45 | + | states := map[entity.PermissionType]webrtcPermissionState{ | |||
| 46 | + | entity.PermissionTypeMicrophone: webrtcPermissionStateIdle, | |||
| 47 | + | entity.PermissionTypeCamera: webrtcPermissionStateIdle, | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | assert.Equal(t, webrtcPermissionStateIdle, summarizeWebRTCPermissionState(states)) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 44 | func TestWebRTCShouldShowIndicator(t *testing.T) { | 53 | func TestWebRTCShouldShowIndicator(t *testing.T) { | ||
| 45 | assert.False(t, shouldShowWebRTCPermissionIndicator(nil)) | 54 | assert.False(t, shouldShowWebRTCPermissionIndicator(nil)) | ||
| 46 | assert.False(t, shouldShowWebRTCPermissionIndicator(map[entity.PermissionType]webrtcPermissionState{ | 55 | assert.False(t, shouldShowWebRTCPermissionIndicator(map[entity.PermissionType]webrtcPermissionState{ | ||
M internal/ui/dialog/permission.go +22 -0
| @@ -129,6 +129,7 @@ func (d *PermissionDialog) showNextQueuedRequest() { | |||
| 129 | 129 | func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) string { | |
| 130 | 130 | hasMic := false | |
| 131 | 131 | hasCam := false | |
| 132 | + | hasDisplay := false | |
| 132 | 133 | ||
| 133 | 134 | for _, pt := range permTypes { | |
| 134 | 135 | switch pt { | |
| @@ -136,16 +137,26 @@ func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) strin | |||
| 136 | 137 | hasMic = true | |
| 137 | 138 | case entity.PermissionTypeCamera: | |
| 138 | 139 | hasCam = true | |
| 140 | + | case entity.PermissionTypeDisplay: | |
| 141 | + | hasDisplay = true | |
| 139 | 142 | } | |
| 140 | 143 | } | |
| 141 | 144 | ||
| 142 | 145 | switch { | |
| 146 | + | case hasMic && hasCam && hasDisplay: | |
| 147 | + | return "Allow Microphone, Camera, and Screen Sharing?" | |
| 148 | + | case hasMic && hasDisplay: | |
| 149 | + | return "Allow Microphone and Screen Sharing?" | |
| 150 | + | case hasCam && hasDisplay: | |
| 151 | + | return "Allow Camera and Screen Sharing?" | |
| 143 | 152 | case hasMic && hasCam: | |
| 144 | 153 | return "Allow Microphone and Camera?" | |
| 145 | 154 | case hasMic: | |
| 146 | 155 | return "Allow Microphone Access?" | |
| 147 | 156 | case hasCam: | |
| 148 | 157 | return "Allow Camera Access?" | |
| 158 | + | case hasDisplay: | |
| 159 | + | return "Allow Screen Sharing?" | |
| 149 | 160 | default: | |
| 150 | 161 | return "Allow Permission?" | |
| 151 | 162 | } | |
| @@ -155,6 +166,7 @@ func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) strin | |||
| 155 | 166 | func (d *PermissionDialog) buildBody(origin string, permTypes []entity.PermissionType) string { | |
| 156 | 167 | hasMic := false | |
| 157 | 168 | hasCam := false | |
| 169 | + | hasDisplay := false | |
| 158 | 170 | ||
| 159 | 171 | for _, pt := range permTypes { | |
| 160 | 172 | switch pt { | |
| @@ -162,17 +174,27 @@ func (d *PermissionDialog) buildBody(origin string, permTypes []entity.Permissio | |||
| 162 | 174 | hasMic = true | |
| 163 | 175 | case entity.PermissionTypeCamera: | |
| 164 | 176 | hasCam = true | |
| 177 | + | case entity.PermissionTypeDisplay: | |
| 178 | + | hasDisplay = true | |
| 165 | 179 | } | |
| 166 | 180 | } | |
| 167 | 181 | ||
| 168 | 182 | var action string | |
| 169 | 183 | switch { | |
| 184 | + | case hasMic && hasCam && hasDisplay: | |
| 185 | + | action = "access your microphone and camera, and share your screen" | |
| 186 | + | case hasMic && hasDisplay: | |
| 187 | + | action = "access your microphone and share your screen" | |
| 188 | + | case hasCam && hasDisplay: | |
| 189 | + | action = "access your camera and share your screen" | |
| 170 | 190 | case hasMic && hasCam: | |
| 171 | 191 | action = "access your microphone and camera" | |
| 172 | 192 | case hasMic: | |
| 173 | 193 | action = "access your microphone" | |
| 174 | 194 | case hasCam: | |
| 175 | 195 | action = "access your camera" | |
| 196 | + | case hasDisplay: | |
| 197 | + | action = "share your screen" | |
| 176 | 198 | default: | |
| 177 | 199 | action = "access your device" | |
| 178 | 200 | } | |
| @@ -129,6 +129,7 @@ func (d *PermissionDialog) showNextQueuedRequest() { | |||||
| 129 | func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) string { | 129 | func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) string { | ||
| 130 | hasMic := false | 130 | hasMic := false | ||
| 131 | hasCam := false | 131 | hasCam := false | ||
| 132 | + | hasDisplay := false | |||
| 132 | 133 | ||||
| 133 | for _, pt := range permTypes { | 134 | for _, pt := range permTypes { | ||
| 134 | switch pt { | 135 | switch pt { | ||
| @@ -136,16 +137,26 @@ func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) strin | |||||
| 136 | hasMic = true | 137 | hasMic = true | ||
| 137 | case entity.PermissionTypeCamera: | 138 | case entity.PermissionTypeCamera: | ||
| 138 | hasCam = true | 139 | hasCam = true | ||
| 140 | + | case entity.PermissionTypeDisplay: | |||
| 141 | + | hasDisplay = true | |||
| 139 | } | 142 | } | ||
| 140 | } | 143 | } | ||
| 141 | 144 | ||||
| 142 | switch { | 145 | switch { | ||
| 146 | + | case hasMic && hasCam && hasDisplay: | |||
| 147 | + | return "Allow Microphone, Camera, and Screen Sharing?" | |||
| 148 | + | case hasMic && hasDisplay: | |||
| 149 | + | return "Allow Microphone and Screen Sharing?" | |||
| 150 | + | case hasCam && hasDisplay: | |||
| 151 | + | return "Allow Camera and Screen Sharing?" | |||
| 143 | case hasMic && hasCam: | 152 | case hasMic && hasCam: | ||
| 144 | return "Allow Microphone and Camera?" | 153 | return "Allow Microphone and Camera?" | ||
| 145 | case hasMic: | 154 | case hasMic: | ||
| 146 | return "Allow Microphone Access?" | 155 | return "Allow Microphone Access?" | ||
| 147 | case hasCam: | 156 | case hasCam: | ||
| 148 | return "Allow Camera Access?" | 157 | return "Allow Camera Access?" | ||
| 158 | + | case hasDisplay: | |||
| 159 | + | return "Allow Screen Sharing?" | |||
| 149 | default: | 160 | default: | ||
| 150 | return "Allow Permission?" | 161 | return "Allow Permission?" | ||
| 151 | } | 162 | } | ||
| @@ -155,6 +166,7 @@ func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) strin | |||||
| 155 | func (d *PermissionDialog) buildBody(origin string, permTypes []entity.PermissionType) string { | 166 | func (d *PermissionDialog) buildBody(origin string, permTypes []entity.PermissionType) string { | ||
| 156 | hasMic := false | 167 | hasMic := false | ||
| 157 | hasCam := false | 168 | hasCam := false | ||
| 169 | + | hasDisplay := false | |||
| 158 | 170 | ||||
| 159 | for _, pt := range permTypes { | 171 | for _, pt := range permTypes { | ||
| 160 | switch pt { | 172 | switch pt { | ||
| @@ -162,17 +174,27 @@ func (d *PermissionDialog) buildBody(origin string, permTypes []entity.Permissio | |||||
| 162 | hasMic = true | 174 | hasMic = true | ||
| 163 | case entity.PermissionTypeCamera: | 175 | case entity.PermissionTypeCamera: | ||
| 164 | hasCam = true | 176 | hasCam = true | ||
| 177 | + | case entity.PermissionTypeDisplay: | |||
| 178 | + | hasDisplay = true | |||
| 165 | } | 179 | } | ||
| 166 | } | 180 | } | ||
| 167 | 181 | ||||
| 168 | var action string | 182 | var action string | ||
| 169 | switch { | 183 | switch { | ||
| 184 | + | case hasMic && hasCam && hasDisplay: | |||
| 185 | + | action = "access your microphone and camera, and share your screen" | |||
| 186 | + | case hasMic && hasDisplay: | |||
| 187 | + | action = "access your microphone and share your screen" | |||
| 188 | + | case hasCam && hasDisplay: | |||
| 189 | + | action = "access your camera and share your screen" | |||
| 170 | case hasMic && hasCam: | 190 | case hasMic && hasCam: | ||
| 171 | action = "access your microphone and camera" | 191 | action = "access your microphone and camera" | ||
| 172 | case hasMic: | 192 | case hasMic: | ||
| 173 | action = "access your microphone" | 193 | action = "access your microphone" | ||
| 174 | case hasCam: | 194 | case hasCam: | ||
| 175 | action = "access your camera" | 195 | action = "access your camera" | ||
| 196 | + | case hasDisplay: | |||
| 197 | + | action = "share your screen" | |||
| 176 | default: | 198 | default: | ||
| 177 | action = "access your device" | 199 | action = "access your device" | ||
| 178 | } | 200 | } | ||
M internal/ui/dialog/permission_test.go +44 -0
| @@ -107,3 +107,47 @@ func TestPermissionDialog_NoPopup_DeniesRequest(t *testing.T) { | |||
| 107 | 107 | assert.True(t, called) | |
| 108 | 108 | assert.Equal(t, port.PermissionDialogResult{Allowed: false, Persistent: false}, result) | |
| 109 | 109 | } | |
| 110 | + | ||
| 111 | + | func TestPermissionDialog_BuildHeadingAndBody_DisplayCombinations(t *testing.T) { | |
| 112 | + | d := &PermissionDialog{} | |
| 113 | + | origin := "https://meet.example.com" | |
| 114 | + | ||
| 115 | + | tests := []struct { | |
| 116 | + | name string | |
| 117 | + | permTypes []entity.PermissionType | |
| 118 | + | expectHeading string | |
| 119 | + | expectedAction string | |
| 120 | + | }{ | |
| 121 | + | { | |
| 122 | + | name: "display only", | |
| 123 | + | permTypes: []entity.PermissionType{entity.PermissionTypeDisplay}, | |
| 124 | + | expectHeading: "Allow Screen Sharing?", | |
| 125 | + | expectedAction: "share your screen", | |
| 126 | + | }, | |
| 127 | + | { | |
| 128 | + | name: "microphone and display", | |
| 129 | + | permTypes: []entity.PermissionType{entity.PermissionTypeMicrophone, entity.PermissionTypeDisplay}, | |
| 130 | + | expectHeading: "Allow Microphone and Screen Sharing?", | |
| 131 | + | expectedAction: "access your microphone and share your screen", | |
| 132 | + | }, | |
| 133 | + | { | |
| 134 | + | name: "camera and display", | |
| 135 | + | permTypes: []entity.PermissionType{entity.PermissionTypeCamera, entity.PermissionTypeDisplay}, | |
| 136 | + | expectHeading: "Allow Camera and Screen Sharing?", | |
| 137 | + | expectedAction: "access your camera and share your screen", | |
| 138 | + | }, | |
| 139 | + | { | |
| 140 | + | name: "all three", | |
| 141 | + | permTypes: []entity.PermissionType{entity.PermissionTypeMicrophone, entity.PermissionTypeCamera, entity.PermissionTypeDisplay}, | |
| 142 | + | expectHeading: "Allow Microphone, Camera, and Screen Sharing?", | |
| 143 | + | expectedAction: "access your microphone and camera, and share your screen", | |
| 144 | + | }, | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | for _, tt := range tests { | |
| 148 | + | t.Run(tt.name, func(t *testing.T) { | |
| 149 | + | assert.Equal(t, tt.expectHeading, d.buildHeading(tt.permTypes)) | |
| 150 | + | assert.Equal(t, origin+" wants to "+tt.expectedAction+".", d.buildBody(origin, tt.permTypes)) | |
| 151 | + | }) | |
| 152 | + | } | |
| 153 | + | } | |
| @@ -107,3 +107,47 @@ func TestPermissionDialog_NoPopup_DeniesRequest(t *testing.T) { | |||||
| 107 | assert.True(t, called) | 107 | assert.True(t, called) | ||
| 108 | assert.Equal(t, port.PermissionDialogResult{Allowed: false, Persistent: false}, result) | 108 | assert.Equal(t, port.PermissionDialogResult{Allowed: false, Persistent: false}, result) | ||
| 109 | } | 109 | } | ||
| 110 | + | ||||
| 111 | + | func TestPermissionDialog_BuildHeadingAndBody_DisplayCombinations(t *testing.T) { | |||
| 112 | + | d := &PermissionDialog{} | |||
| 113 | + | origin := "https://meet.example.com" | |||
| 114 | + | ||||
| 115 | + | tests := []struct { | |||
| 116 | + | name string | |||
| 117 | + | permTypes []entity.PermissionType | |||
| 118 | + | expectHeading string | |||
| 119 | + | expectedAction string | |||
| 120 | + | }{ | |||
| 121 | + | { | |||
| 122 | + | name: "display only", | |||
| 123 | + | permTypes: []entity.PermissionType{entity.PermissionTypeDisplay}, | |||
| 124 | + | expectHeading: "Allow Screen Sharing?", | |||
| 125 | + | expectedAction: "share your screen", | |||
| 126 | + | }, | |||
| 127 | + | { | |||
| 128 | + | name: "microphone and display", | |||
| 129 | + | permTypes: []entity.PermissionType{entity.PermissionTypeMicrophone, entity.PermissionTypeDisplay}, | |||
| 130 | + | expectHeading: "Allow Microphone and Screen Sharing?", | |||
| 131 | + | expectedAction: "access your microphone and share your screen", | |||
| 132 | + | }, | |||
| 133 | + | { | |||
| 134 | + | name: "camera and display", | |||
| 135 | + | permTypes: []entity.PermissionType{entity.PermissionTypeCamera, entity.PermissionTypeDisplay}, | |||
| 136 | + | expectHeading: "Allow Camera and Screen Sharing?", | |||
| 137 | + | expectedAction: "access your camera and share your screen", | |||
| 138 | + | }, | |||
| 139 | + | { | |||
| 140 | + | name: "all three", | |||
| 141 | + | permTypes: []entity.PermissionType{entity.PermissionTypeMicrophone, entity.PermissionTypeCamera, entity.PermissionTypeDisplay}, | |||
| 142 | + | expectHeading: "Allow Microphone, Camera, and Screen Sharing?", | |||
| 143 | + | expectedAction: "access your microphone and camera, and share your screen", | |||
| 144 | + | }, | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | for _, tt := range tests { | |||
| 148 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 149 | + | assert.Equal(t, tt.expectHeading, d.buildHeading(tt.permTypes)) | |||
| 150 | + | assert.Equal(t, origin+" wants to "+tt.expectedAction+".", d.buildBody(origin, tt.permTypes)) | |||
| 151 | + | }) | |||
| 152 | + | } | |||
| 153 | + | } | |||
M internal/ui/theme/webrtc_indicator_css.go +1 -9
| @@ -46,11 +46,7 @@ func generateWebRTCPermissionIndicatorCSS(p Palette) string { | |||
| 46 | 46 | box-shadow 200ms ease-in-out; | |
| 47 | 47 | } | |
| 48 | 48 | ||
| 49 | - | /* Header row: origin label (left) + dot (right) on the same line */ | |
| 50 | - | .webrtc-indicator-header { | |
| 51 | - | } | |
| 52 | - | ||
| 53 | - | /* Origin label - shown next to the dot, hidden when collapsed (no text) */ | |
| 49 | + | /* Origin label shown next to the dot. */ | |
| 54 | 50 | .webrtc-indicator-origin { | |
| 55 | 51 | font-size: 0.6875em; | |
| 56 | 52 | font-weight: 600; | |
| @@ -59,10 +55,6 @@ func generateWebRTCPermissionIndicatorCSS(p Palette) string { | |||
| 59 | 55 | color: var(--muted); | |
| 60 | 56 | } | |
| 61 | 57 | ||
| 62 | - | /* GtkRevealer wrapper */ | |
| 63 | - | .webrtc-indicator-revealer { | |
| 64 | - | } | |
| 65 | - | ||
| 66 | 58 | /* Details section inside the revealer (permission rows only) */ | |
| 67 | 59 | .webrtc-indicator-details { | |
| 68 | 60 | margin-top: 0.3em; | |
| @@ -46,11 +46,7 @@ func generateWebRTCPermissionIndicatorCSS(p Palette) string { | |||||
| 46 | box-shadow 200ms ease-in-out; | 46 | box-shadow 200ms ease-in-out; | ||
| 47 | } | 47 | } | ||
| 48 | 48 | ||||
| 49 | - | /* Header row: origin label (left) + dot (right) on the same line */ | 49 | + | /* Origin label shown next to the dot. */ |
| 50 | - | .webrtc-indicator-header { | |||
| 51 | - | } | |||
| 52 | - | ||||
| 53 | - | /* Origin label - shown next to the dot, hidden when collapsed (no text) */ | |||
| 54 | .webrtc-indicator-origin { | 50 | .webrtc-indicator-origin { | ||
| 55 | font-size: 0.6875em; | 51 | font-size: 0.6875em; | ||
| 56 | font-weight: 600; | 52 | font-weight: 600; | ||
| @@ -59,10 +55,6 @@ func generateWebRTCPermissionIndicatorCSS(p Palette) string { | |||||
| 59 | color: var(--muted); | 55 | color: var(--muted); | ||
| 60 | } | 56 | } | ||
| 61 | 57 | ||||
| 62 | - | /* GtkRevealer wrapper */ | |||
| 63 | - | .webrtc-indicator-revealer { | |||
| 64 | - | } | |||
| 65 | - | ||||
| 66 | /* Details section inside the revealer (permission rows only) */ | 58 | /* Details section inside the revealer (permission rows only) */ | ||
| 67 | .webrtc-indicator-details { | 59 | .webrtc-indicator-details { | ||
| 68 | margin-top: 0.3em; | 60 | margin-top: 0.3em; | ||