feat(release): setup goreleaser with Docker-based workflow
- Add .goreleaser.yaml configuration for automated release builds - Add Dockerfile.release and docker-compose.yml for containerized builds - Add docker-check, docker-build, docker-release make targets - Switch from syscall.Dup2 to unix.Dup3 for better cross-platform support - Update GPU driver recommendations in README - Remove completed POPUP_REFACTORING_PLAN.md
- commit
- 7b2e5718f022ca9cfacb21488862ab4abde8e647
- author
- bnema · · 2025-10-16 05:21 UTC
- parent
- d28477f
- signature
- unsigned
7 files · +257 -660
A .goreleaser.yaml +183 -0
| @@ -0,0 +1,183 @@ | |||
| 1 | + | # GoReleaser configuration for dumber browser | |
| 2 | + | # https://goreleaser.com | |
| 3 | + | version: 2 | |
| 4 | + | ||
| 5 | + | before: | |
| 6 | + | hooks: | |
| 7 | + | # Build frontend assets before building binaries | |
| 8 | + | - make build-frontend | |
| 9 | + | ||
| 10 | + | builds: | |
| 11 | + | # Main build with CGO for WebKitGTK | |
| 12 | + | - id: dumber | |
| 13 | + | main: . | |
| 14 | + | binary: dumber | |
| 15 | + | ||
| 16 | + | env: | |
| 17 | + | - CGO_ENABLED=1 | |
| 18 | + | ||
| 19 | + | goos: | |
| 20 | + | - linux | |
| 21 | + | ||
| 22 | + | goarch: | |
| 23 | + | - amd64 | |
| 24 | + | - arm64 | |
| 25 | + | ||
| 26 | + | # Version information via ldflags (matching your Makefile) | |
| 27 | + | ldflags: | |
| 28 | + | - -s -w | |
| 29 | + | - -X main.version={{.Version}} | |
| 30 | + | - -X main.commit={{.ShortCommit}} | |
| 31 | + | - -X main.buildDate={{.Date}} | |
| 32 | + | ||
| 33 | + | # Build tags for WebKitGTK | |
| 34 | + | tags: | |
| 35 | + | - webkit_cgo | |
| 36 | + | ||
| 37 | + | # CGO cross-compilation requires proper toolchain setup | |
| 38 | + | # For local builds, you'll need cross-compilers installed | |
| 39 | + | # For CI/CD, use goreleaser-cross Docker images | |
| 40 | + | overrides: | |
| 41 | + | - goos: linux | |
| 42 | + | goarch: amd64 | |
| 43 | + | env: | |
| 44 | + | - CC=x86_64-linux-gnu-gcc | |
| 45 | + | - CXX=x86_64-linux-gnu-g++ | |
| 46 | + | - goos: linux | |
| 47 | + | goarch: arm64 | |
| 48 | + | env: | |
| 49 | + | - CC=aarch64-linux-gnu-gcc | |
| 50 | + | - CXX=aarch64-linux-gnu-g++ | |
| 51 | + | ||
| 52 | + | archives: | |
| 53 | + | - id: default | |
| 54 | + | name_template: >- | |
| 55 | + | {{ .ProjectName }}_ | |
| 56 | + | {{- .Version }}_ | |
| 57 | + | {{- .Os }}_ | |
| 58 | + | {{- if eq .Arch "amd64" }}x86_64 | |
| 59 | + | {{- else if eq .Arch "386" }}i386 | |
| 60 | + | {{- else }}{{ .Arch }}{{ end }} | |
| 61 | + | format_overrides: | |
| 62 | + | - goos: linux | |
| 63 | + | format: tar.gz | |
| 64 | + | files: | |
| 65 | + | - LICENSE | |
| 66 | + | - README.md | |
| 67 | + | - CHANGELOG.md | |
| 68 | + | ||
| 69 | + | checksum: | |
| 70 | + | name_template: 'checksums.txt' | |
| 71 | + | algorithm: sha256 | |
| 72 | + | ||
| 73 | + | snapshot: | |
| 74 | + | version_template: "{{ incpatch .Version }}-next" | |
| 75 | + | ||
| 76 | + | changelog: | |
| 77 | + | sort: asc | |
| 78 | + | use: github | |
| 79 | + | filters: | |
| 80 | + | exclude: | |
| 81 | + | - '^docs:' | |
| 82 | + | - '^test:' | |
| 83 | + | - '^ci:' | |
| 84 | + | - '^chore:' | |
| 85 | + | - '^build\(deps\):' | |
| 86 | + | - 'Merge pull request' | |
| 87 | + | - 'Merge branch' | |
| 88 | + | groups: | |
| 89 | + | - title: 'Features' | |
| 90 | + | regexp: '^feat(\(.+\))?:' | |
| 91 | + | order: 0 | |
| 92 | + | - title: 'Bug Fixes' | |
| 93 | + | regexp: '^fix(\(.+\))?:' | |
| 94 | + | order: 1 | |
| 95 | + | - title: 'Performance' | |
| 96 | + | regexp: '^perf(\(.+\))?:' | |
| 97 | + | order: 2 | |
| 98 | + | - title: 'Refactoring' | |
| 99 | + | regexp: '^refactor(\(.+\))?:' | |
| 100 | + | order: 3 | |
| 101 | + | - title: 'Documentation' | |
| 102 | + | regexp: '^docs(\(.+\))?:' | |
| 103 | + | order: 4 | |
| 104 | + | - title: 'Other Changes' | |
| 105 | + | order: 999 | |
| 106 | + | ||
| 107 | + | release: | |
| 108 | + | github: | |
| 109 | + | owner: bnema | |
| 110 | + | name: dumber | |
| 111 | + | ||
| 112 | + | # Don't auto-publish as draft initially | |
| 113 | + | draft: false | |
| 114 | + | ||
| 115 | + | # Replace existing draft if it exists | |
| 116 | + | replace_existing_draft: true | |
| 117 | + | ||
| 118 | + | # Header for release notes | |
| 119 | + | header: | | |
| 120 | + | ## New Release {{.Tag}} | |
| 121 | + | ||
| 122 | + | **Built with Go {{.Env.GOVERSION | default "1.25.1"}}** on {{.Date}} | |
| 123 | + | ||
| 124 | + | # Footer with installation instructions | |
| 125 | + | footer: | | |
| 126 | + | --- | |
| 127 | + | ||
| 128 | + | ## Installation | |
| 129 | + | ||
| 130 | + | ### Linux (x86_64/amd64) | |
| 131 | + | ```bash | |
| 132 | + | wget https://github.com/bnema/dumber/releases/download/{{.Tag}}/dumber_{{.Version}}_linux_x86_64.tar.gz | |
| 133 | + | tar -xzf dumber_{{.Version}}_linux_x86_64.tar.gz | |
| 134 | + | sudo install -m 755 dumber /usr/local/bin/ | |
| 135 | + | ``` | |
| 136 | + | ||
| 137 | + | ### Linux (arm64) | |
| 138 | + | ```bash | |
| 139 | + | wget https://github.com/bnema/dumber/releases/download/{{.Tag}}/dumber_{{.Version}}_linux_arm64.tar.gz | |
| 140 | + | tar -xzf dumber_{{.Version}}_linux_arm64.tar.gz | |
| 141 | + | sudo install -m 755 dumber /usr/local/bin/ | |
| 142 | + | ``` | |
| 143 | + | ||
| 144 | + | ### Dependencies | |
| 145 | + | ||
| 146 | + | **Arch Linux:** | |
| 147 | + | ```bash | |
| 148 | + | # Core WebKitGTK 6.0 and GTK4 | |
| 149 | + | sudo pacman -S webkitgtk-6.0 gtk4 | |
| 150 | + | ||
| 151 | + | # GStreamer for media playback | |
| 152 | + | sudo pacman -S gst-plugins-base gst-plugins-good gst-plugins-bad gst-plugins-ugly gst-libav gst-plugin-pipewire pipewire pipewire-pulse | |
| 153 | + | ||
| 154 | + | # Hardware acceleration (choose based on your GPU) | |
| 155 | + | # AMD: sudo pacman -S gstreamer-vaapi mesa | |
| 156 | + | # NVIDIA: sudo pacman -S gstreamer-vaapi libva-nvidia-driver | |
| 157 | + | # Intel: sudo pacman -S gstreamer-vaapi libva-intel-driver intel-media-driver | |
| 158 | + | ``` | |
| 159 | + | ||
| 160 | + | **Debian/Ubuntu:** | |
| 161 | + | ```bash | |
| 162 | + | # Core WebKitGTK and GTK4 | |
| 163 | + | sudo apt install libwebkitgtk-6.0-4 libgtk-4-1 | |
| 164 | + | ||
| 165 | + | # GStreamer for media playback | |
| 166 | + | sudo apt install gstreamer1.0-tools gstreamer1.0-plugins-base gstreamer1.0-plugins-good gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly gstreamer1.0-libav gstreamer1.0-pipewire | |
| 167 | + | ||
| 168 | + | # Hardware acceleration | |
| 169 | + | sudo apt install gstreamer1.0-vaapi va-driver-all | |
| 170 | + | ``` | |
| 171 | + | ||
| 172 | + | --- | |
| 173 | + | ||
| 174 | + | **Full Changelog**: https://github.com/bnema/dumber/compare/{{.PreviousTag}}...{{.Tag}} | |
| 175 | + | ||
| 176 | + | # Auto-detect prerelease from version (v1.0.0-rc1, v1.0.0-alpha, etc.) | |
| 177 | + | prerelease: auto | |
| 178 | + | ||
| 179 | + | # Mark as latest release | |
| 180 | + | make_latest: true | |
| 181 | + | ||
| 182 | + | # Replace existing release notes | |
| 183 | + | mode: replace | |
| @@ -0,0 +1,183 @@ | |||||
| 1 | + | # GoReleaser configuration for dumber browser | |||
| 2 | + | # https://goreleaser.com | |||
| 3 | + | version: 2 | |||
| 4 | + | ||||
| 5 | + | before: | |||
| 6 | + | hooks: | |||
| 7 | + | # Build frontend assets before building binaries | |||
| 8 | + | - make build-frontend | |||
| 9 | + | ||||
| 10 | + | builds: | |||
| 11 | + | # Main build with CGO for WebKitGTK | |||
| 12 | + | - id: dumber | |||
| 13 | + | main: . | |||
| 14 | + | binary: dumber | |||
| 15 | + | ||||
| 16 | + | env: | |||
| 17 | + | - CGO_ENABLED=1 | |||
| 18 | + | ||||
| 19 | + | goos: | |||
| 20 | + | - linux | |||
| 21 | + | ||||
| 22 | + | goarch: | |||
| 23 | + | - amd64 | |||
| 24 | + | - arm64 | |||
| 25 | + | ||||
| 26 | + | # Version information via ldflags (matching your Makefile) | |||
| 27 | + | ldflags: | |||
| 28 | + | - -s -w | |||
| 29 | + | - -X main.version={{.Version}} | |||
| 30 | + | - -X main.commit={{.ShortCommit}} | |||
| 31 | + | - -X main.buildDate={{.Date}} | |||
| 32 | + | ||||
| 33 | + | # Build tags for WebKitGTK | |||
| 34 | + | tags: | |||
| 35 | + | - webkit_cgo | |||
| 36 | + | ||||
| 37 | + | # CGO cross-compilation requires proper toolchain setup | |||
| 38 | + | # For local builds, you'll need cross-compilers installed | |||
| 39 | + | # For CI/CD, use goreleaser-cross Docker images | |||
| 40 | + | overrides: | |||
| 41 | + | - goos: linux | |||
| 42 | + | goarch: amd64 | |||
| 43 | + | env: | |||
| 44 | + | - CC=x86_64-linux-gnu-gcc | |||
| 45 | + | - CXX=x86_64-linux-gnu-g++ | |||
| 46 | + | - goos: linux | |||
| 47 | + | goarch: arm64 | |||
| 48 | + | env: | |||
| 49 | + | - CC=aarch64-linux-gnu-gcc | |||
| 50 | + | - CXX=aarch64-linux-gnu-g++ | |||
| 51 | + | ||||
| 52 | + | archives: | |||
| 53 | + | - id: default | |||
| 54 | + | name_template: >- | |||
| 55 | + | {{ .ProjectName }}_ | |||
| 56 | + | {{- .Version }}_ | |||
| 57 | + | {{- .Os }}_ | |||
| 58 | + | {{- if eq .Arch "amd64" }}x86_64 | |||
| 59 | + | {{- else if eq .Arch "386" }}i386 | |||
| 60 | + | {{- else }}{{ .Arch }}{{ end }} | |||
| 61 | + | format_overrides: | |||
| 62 | + | - goos: linux | |||
| 63 | + | format: tar.gz | |||
| 64 | + | files: | |||
| 65 | + | - LICENSE | |||
| 66 | + | - README.md | |||
| 67 | + | - CHANGELOG.md | |||
| 68 | + | ||||
| 69 | + | checksum: | |||
| 70 | + | name_template: 'checksums.txt' | |||
| 71 | + | algorithm: sha256 | |||
| 72 | + | ||||
| 73 | + | snapshot: | |||
| 74 | + | version_template: "{{ incpatch .Version }}-next" | |||
| 75 | + | ||||
| 76 | + | changelog: | |||
| 77 | + | sort: asc | |||
| 78 | + | use: github | |||
| 79 | + | filters: | |||
| 80 | + | exclude: | |||
| 81 | + | - '^docs:' | |||
| 82 | + | - '^test:' | |||
| 83 | + | - '^ci:' | |||
| 84 | + | - '^chore:' | |||
| 85 | + | - '^build\(deps\):' | |||
| 86 | + | - 'Merge pull request' | |||
| 87 | + | - 'Merge branch' | |||
| 88 | + | groups: | |||
| 89 | + | - title: 'Features' | |||
| 90 | + | regexp: '^feat(\(.+\))?:' | |||
| 91 | + | order: 0 | |||
| 92 | + | - title: 'Bug Fixes' | |||
| 93 | + | regexp: '^fix(\(.+\))?:' | |||
| 94 | + | order: 1 | |||
| 95 | + | - title: 'Performance' | |||
| 96 | + | regexp: '^perf(\(.+\))?:' | |||
| 97 | + | order: 2 | |||
| 98 | + | - title: 'Refactoring' | |||
| 99 | + | regexp: '^refactor(\(.+\))?:' | |||
| 100 | + | order: 3 | |||
| 101 | + | - title: 'Documentation' | |||
| 102 | + | regexp: '^docs(\(.+\))?:' | |||
| 103 | + | order: 4 | |||
| 104 | + | - title: 'Other Changes' | |||
| 105 | + | order: 999 | |||
| 106 | + | ||||
| 107 | + | release: | |||
| 108 | + | github: | |||
| 109 | + | owner: bnema | |||
| 110 | + | name: dumber | |||
| 111 | + | ||||
| 112 | + | # Don't auto-publish as draft initially | |||
| 113 | + | draft: false | |||
| 114 | + | ||||
| 115 | + | # Replace existing draft if it exists | |||
| 116 | + | replace_existing_draft: true | |||
| 117 | + | ||||
| 118 | + | # Header for release notes | |||
| 119 | + | header: | | |||
| 120 | + | ## New Release {{.Tag}} | |||
| 121 | + | ||||
| 122 | + | **Built with Go {{.Env.GOVERSION | default "1.25.1"}}** on {{.Date}} | |||
| 123 | + | ||||
| 124 | + | # Footer with installation instructions | |||
| 125 | + | footer: | | |||
| 126 | + | --- | |||
| 127 | + | ||||
| 128 | + | ## Installation | |||
| 129 | + | ||||
| 130 | + | ### Linux (x86_64/amd64) | |||
| 131 | + | ```bash | |||
| 132 | + | wget https://github.com/bnema/dumber/releases/download/{{.Tag}}/dumber_{{.Version}}_linux_x86_64.tar.gz | |||
| 133 | + | tar -xzf dumber_{{.Version}}_linux_x86_64.tar.gz | |||
| 134 | + | sudo install -m 755 dumber /usr/local/bin/ | |||
| 135 | + | ``` | |||
| 136 | + | ||||
| 137 | + | ### Linux (arm64) | |||
| 138 | + | ```bash | |||
| 139 | + | wget https://github.com/bnema/dumber/releases/download/{{.Tag}}/dumber_{{.Version}}_linux_arm64.tar.gz | |||
| 140 | + | tar -xzf dumber_{{.Version}}_linux_arm64.tar.gz | |||
| 141 | + | sudo install -m 755 dumber /usr/local/bin/ | |||
| 142 | + | ``` | |||
| 143 | + | ||||
| 144 | + | ### Dependencies | |||
| 145 | + | ||||
| 146 | + | **Arch Linux:** | |||
| 147 | + | ```bash | |||
| 148 | + | # Core WebKitGTK 6.0 and GTK4 | |||
| 149 | + | sudo pacman -S webkitgtk-6.0 gtk4 | |||
| 150 | + | ||||
| 151 | + | # GStreamer for media playback | |||
| 152 | + | sudo pacman -S gst-plugins-base gst-plugins-good gst-plugins-bad gst-plugins-ugly gst-libav gst-plugin-pipewire pipewire pipewire-pulse | |||
| 153 | + | ||||
| 154 | + | # Hardware acceleration (choose based on your GPU) | |||
| 155 | + | # AMD: sudo pacman -S gstreamer-vaapi mesa | |||
| 156 | + | # NVIDIA: sudo pacman -S gstreamer-vaapi libva-nvidia-driver | |||
| 157 | + | # Intel: sudo pacman -S gstreamer-vaapi libva-intel-driver intel-media-driver | |||
| 158 | + | ``` | |||
| 159 | + | ||||
| 160 | + | **Debian/Ubuntu:** | |||
| 161 | + | ```bash | |||
| 162 | + | # Core WebKitGTK and GTK4 | |||
| 163 | + | sudo apt install libwebkitgtk-6.0-4 libgtk-4-1 | |||
| 164 | + | ||||
| 165 | + | # GStreamer for media playback | |||
| 166 | + | sudo apt install gstreamer1.0-tools gstreamer1.0-plugins-base gstreamer1.0-plugins-good gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly gstreamer1.0-libav gstreamer1.0-pipewire | |||
| 167 | + | ||||
| 168 | + | # Hardware acceleration | |||
| 169 | + | sudo apt install gstreamer1.0-vaapi va-driver-all | |||
| 170 | + | ``` | |||
| 171 | + | ||||
| 172 | + | --- | |||
| 173 | + | ||||
| 174 | + | **Full Changelog**: https://github.com/bnema/dumber/compare/{{.PreviousTag}}...{{.Tag}} | |||
| 175 | + | ||||
| 176 | + | # Auto-detect prerelease from version (v1.0.0-rc1, v1.0.0-alpha, etc.) | |||
| 177 | + | prerelease: auto | |||
| 178 | + | ||||
| 179 | + | # Mark as latest release | |||
| 180 | + | make_latest: true | |||
| 181 | + | ||||
| 182 | + | # Replace existing release notes | |||
| 183 | + | mode: replace | |||
A Dockerfile.release +17 -0
| @@ -0,0 +1,17 @@ | |||
| 1 | + | # Dockerfile for building releases with goreleaser | |
| 2 | + | # Uses goreleaser-cross which includes cross-compilers for CGO builds | |
| 3 | + | FROM ghcr.io/goreleaser/goreleaser-cross:latest | |
| 4 | + | ||
| 5 | + | # Install Node.js and npm for building frontend | |
| 6 | + | RUN apt-get update && \ | |
| 7 | + | apt-get install -y curl && \ | |
| 8 | + | curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && \ | |
| 9 | + | apt-get install -y nodejs && \ | |
| 10 | + | apt-get clean && \ | |
| 11 | + | rm -rf /var/lib/apt/lists/* | |
| 12 | + | ||
| 13 | + | # Set working directory | |
| 14 | + | WORKDIR /workspace | |
| 15 | + | ||
| 16 | + | # Default command | |
| 17 | + | CMD ["goreleaser", "release", "--clean"] | |
| @@ -0,0 +1,17 @@ | |||||
| 1 | + | # Dockerfile for building releases with goreleaser | |||
| 2 | + | # Uses goreleaser-cross which includes cross-compilers for CGO builds | |||
| 3 | + | FROM ghcr.io/goreleaser/goreleaser-cross:latest | |||
| 4 | + | ||||
| 5 | + | # Install Node.js and npm for building frontend | |||
| 6 | + | RUN apt-get update && \ | |||
| 7 | + | apt-get install -y curl && \ | |||
| 8 | + | curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && \ | |||
| 9 | + | apt-get install -y nodejs && \ | |||
| 10 | + | apt-get clean && \ | |||
| 11 | + | rm -rf /var/lib/apt/lists/* | |||
| 12 | + | ||||
| 13 | + | # Set working directory | |||
| 14 | + | WORKDIR /workspace | |||
| 15 | + | ||||
| 16 | + | # Default command | |||
| 17 | + | CMD ["goreleaser", "release", "--clean"] | |||
M Makefile +12 -0
| @@ -148,3 +148,15 @@ check: ## Check that all tools and dependencies are working | |||
| 148 | 148 | @echo "\nRunning tests..." | |
| 149 | 149 | @$(MAKE) test | |
| 150 | 150 | @echo "\n✅ All checks passed! Project is ready for development." | |
| 151 | + | ||
| 152 | + | # Docker-based release targets | |
| 153 | + | .PHONY: docker-build docker-release docker-check | |
| 154 | + | ||
| 155 | + | docker-check: ## Check goreleaser config using Docker | |
| 156 | + | docker compose run --rm check | |
| 157 | + | ||
| 158 | + | docker-build: ## Build snapshot using Docker (no git tags required) | |
| 159 | + | GITHUB_TOKEN=$$(gh auth token) docker compose run --rm goreleaser | |
| 160 | + | ||
| 161 | + | docker-release: ## Create full release using Docker | |
| 162 | + | GITHUB_TOKEN=$$(gh auth token) docker compose run --rm release | |
| @@ -148,3 +148,15 @@ check: ## Check that all tools and dependencies are working | |||||
| 148 | @echo "\nRunning tests..." | 148 | @echo "\nRunning tests..." | ||
| 149 | @$(MAKE) test | 149 | @$(MAKE) test | ||
| 150 | @echo "\n✅ All checks passed! Project is ready for development." | 150 | @echo "\n✅ All checks passed! Project is ready for development." | ||
| 151 | + | ||||
| 152 | + | # Docker-based release targets | |||
| 153 | + | .PHONY: docker-build docker-release docker-check | |||
| 154 | + | ||||
| 155 | + | docker-check: ## Check goreleaser config using Docker | |||
| 156 | + | docker compose run --rm check | |||
| 157 | + | ||||
| 158 | + | docker-build: ## Build snapshot using Docker (no git tags required) | |||
| 159 | + | GITHUB_TOKEN=$$(gh auth token) docker compose run --rm goreleaser | |||
| 160 | + | ||||
| 161 | + | docker-release: ## Create full release using Docker | |||
| 162 | + | GITHUB_TOKEN=$$(gh auth token) docker compose run --rm release | |||
D POPUP_REFACTORING_PLAN.md +0 -654
| @@ -1,654 +0,0 @@ | |||
| 1 | - | # Popup Window Refactoring Plan | |
| 2 | - | ||
| 3 | - | ## Executive Summary | |
| 4 | - | ||
| 5 | - | The current popup window implementation causes critical SIGSEGV crashes due to improper lifecycle management that conflicts with WebKitGTK's internal architecture. This document outlines the issues, root causes, and a comprehensive refactoring plan to align with WebKitGTK's expected signal-based popup lifecycle. | |
| 6 | - | ||
| 7 | - | ## Issues Faced | |
| 8 | - | ||
| 9 | - | ### 1. Critical Crash: SIGSEGV on Popup Close | |
| 10 | - | ||
| 11 | - | **Symptom:** | |
| 12 | - | ``` | |
| 13 | - | signal 11 received but handler not on signal stack | |
| 14 | - | fatal error: non-Go code set up signal handler without SA_ONSTACK flag | |
| 15 | - | ``` | |
| 16 | - | ||
| 17 | - | **Occurrences:** | |
| 18 | - | - OAuth popup completion (Google, Notion login) | |
| 19 | - | - Rapid popup open/close cycles (popup blocker detection) | |
| 20 | - | - Any programmatic popup closure | |
| 21 | - | ||
| 22 | - | **Impact:** Browser crash, data loss, poor user experience | |
| 23 | - | ||
| 24 | - | ### 2. GTK Bloom Filter Corruption | |
| 25 | - | ||
| 26 | - | **Symptom:** | |
| 27 | - | ``` | |
| 28 | - | [workspace] Skipping CSS, pane 0xc00015c000 reparented 0.6ms ago | |
| 29 | - | ``` | |
| 30 | - | ||
| 31 | - | **Root Cause:** CSS class modifications during widget reparenting operations before GTK widget hierarchy stabilizes | |
| 32 | - | ||
| 33 | - | **Attempted Fixes (all failed):** | |
| 34 | - | - Boolean flag `pendingReparent` cleared too early | |
| 35 | - | - Timestamp-based CSS blocking (150ms safety window) | |
| 36 | - | - `StopLoading()` before close (broke OAuth callback) | |
| 37 | - | ||
| 38 | - | ### 3. WebKit/GTK Race Conditions | |
| 39 | - | ||
| 40 | - | **Manifestations:** | |
| 41 | - | - localStorage JS injection immediately before `ClosePane()` triggered WebKit JS engine during GTK reparenting | |
| 42 | - | - Rapid popup opens (<1s) caused WebKit rendering pipeline initialization conflicts with GTK widget destruction | |
| 43 | - | - WebKit signal handlers firing after GTK widgets destroyed | |
| 44 | - | ||
| 45 | - | ## Root Cause Analysis | |
| 46 | - | ||
| 47 | - | ### Architecture Violation: Bypassing WebKit Popup Lifecycle | |
| 48 | - | ||
| 49 | - | **Current Implementation Flow:** | |
| 50 | - | ``` | |
| 51 | - | JavaScript window.open() | |
| 52 | - | ↓ | |
| 53 | - | main-world.js intercepts | |
| 54 | - | ↓ | |
| 55 | - | postMessage to Go | |
| 56 | - | ↓ | |
| 57 | - | messaging.Handler | |
| 58 | - | ↓ | |
| 59 | - | WorkspaceManager.handleIntentAsPopup() | |
| 60 | - | ↓ | |
| 61 | - | Manual WebView creation | |
| 62 | - | ↓ | |
| 63 | - | Manual pane insertion | |
| 64 | - | ↓ | |
| 65 | - | Manual close (ClosePane) | |
| 66 | - | ``` | |
| 67 | - | ||
| 68 | - | **Problem:** This completely bypasses WebKit's native popup management system. | |
| 69 | - | ||
| 70 | - | ### WebKitGTK's Expected Popup Lifecycle | |
| 71 | - | ||
| 72 | - | **Reference:** `/usr/include/webkitgtk-6.0/webkit/WebKitWebView.h:243-259` | |
| 73 | - | ||
| 74 | - | ```c | |
| 75 | - | struct _WebKitWebViewClass { | |
| 76 | - | // ... other signals ... | |
| 77 | - | ||
| 78 | - | GtkWidget *(* create) (WebKitWebView *web_view, | |
| 79 | - | WebKitNavigationAction *navigation_action); | |
| 80 | - | ||
| 81 | - | void (* ready_to_show) (WebKitWebView *web_view); | |
| 82 | - | ||
| 83 | - | void (* close) (WebKitWebView *web_view); | |
| 84 | - | ||
| 85 | - | // ... other signals ... | |
| 86 | - | }; | |
| 87 | - | ``` | |
| 88 | - | ||
| 89 | - | **WebKit's Lifecycle Contract:** | |
| 90 | - | ||
| 91 | - | 1. **`create` signal**: WebKit requests new WebView for popup | |
| 92 | - | - Application creates and configures new WebView | |
| 93 | - | - Application returns WebView to WebKit | |
| 94 | - | - **WebKit takes ownership of loading and content rendering** | |
| 95 | - | ||
| 96 | - | 2. **`ready-to-show` signal**: WebKit indicates popup is ready | |
| 97 | - | - WebKit has finished initial setup | |
| 98 | - | - Safe to show/position window/pane | |
| 99 | - | - Window properties (size, features) are available | |
| 100 | - | ||
| 101 | - | 3. **`close` signal**: WebKit requests popup closure | |
| 102 | - | - Emitted when JavaScript calls `window.close()` | |
| 103 | - | - Emitted when page navigates away from popup context | |
| 104 | - | - **Application should destroy widgets in response** | |
| 105 | - | ||
| 106 | - | ### Why Current Implementation Fails | |
| 107 | - | ||
| 108 | - | 1. **No signal connection**: Application never connects to WebKit's `create` signal | |
| 109 | - | 2. **Manual lifecycle**: Application creates WebViews independently, breaking WebKit's internal reference tracking | |
| 110 | - | 3. **Premature destruction**: `ClosePane()` destroys GTK widgets while WebKit still has: | |
| 111 | - | - Active signal handlers | |
| 112 | - | - Compositor threads running | |
| 113 | - | - Rendering pipeline active | |
| 114 | - | - JS engine references | |
| 115 | - | ||
| 116 | - | 4. **Race conditions**: JavaScript message handling races with WebKit's internal operations | |
| 117 | - | ||
| 118 | - | ## Reference Implementations | |
| 119 | - | ||
| 120 | - | ### Epiphany (GNOME Web Browser) | |
| 121 | - | ||
| 122 | - | **File:** `/home/brice/dev/clone/epiphany/src/ephy-window.c` | |
| 123 | - | ||
| 124 | - | **Key Implementation Points:** | |
| 125 | - | ||
| 126 | - | 1. **Signal Connection** (line 2636-2638): | |
| 127 | - | ```c | |
| 128 | - | g_signal_connect_object (web_view, "create", | |
| 129 | - | G_CALLBACK (create_web_view_cb), | |
| 130 | - | window, 0); | |
| 131 | - | ``` | |
| 132 | - | ||
| 133 | - | 2. **Create Handler** (lines 2034-2070): | |
| 134 | - | ```c | |
| 135 | - | static WebKitWebView * | |
| 136 | - | create_web_view_cb (WebKitWebView *web_view, | |
| 137 | - | WebKitNavigationAction *navigation_action, | |
| 138 | - | EphyWindow *window) | |
| 139 | - | { | |
| 140 | - | EphyEmbed *embed; | |
| 141 | - | WebKitWebView *new_web_view; | |
| 142 | - | ||
| 143 | - | // Determine if popup should be new window or new tab | |
| 144 | - | if (new_windows_in_tabs_setting) { | |
| 145 | - | target_window = window; // Same window (tab) | |
| 146 | - | } else { | |
| 147 | - | target_window = ephy_window_new (); // New window | |
| 148 | - | } | |
| 149 | - | ||
| 150 | - | // Create new tab/window with parent relationship | |
| 151 | - | embed = ephy_shell_new_tab_full (..., web_view, ...); | |
| 152 | - | ||
| 153 | - | new_web_view = EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED (embed); | |
| 154 | - | ||
| 155 | - | // Connect ready-to-show handler | |
| 156 | - | g_signal_connect_object (new_web_view, "ready-to-show", | |
| 157 | - | G_CALLBACK (web_view_ready_cb), | |
| 158 | - | web_view, 0); | |
| 159 | - | ||
| 160 | - | // Return new WebView to WebKit | |
| 161 | - | return new_web_view; | |
| 162 | - | } | |
| 163 | - | ``` | |
| 164 | - | ||
| 165 | - | 3. **Ready-to-Show Handler** (lines 2012-2031): | |
| 166 | - | ```c | |
| 167 | - | static gboolean | |
| 168 | - | web_view_ready_cb (WebKitWebView *web_view, | |
| 169 | - | WebKitWebView *parent_web_view) | |
| 170 | - | { | |
| 171 | - | EphyWindow *window; | |
| 172 | - | ||
| 173 | - | window = EPHY_WINDOW (gtk_widget_get_root (GTK_WIDGET (web_view))); | |
| 174 | - | ||
| 175 | - | // Apply window properties | |
| 176 | - | if (using_new_window) { | |
| 177 | - | ephy_window_configure_for_view (window, web_view); | |
| 178 | - | } | |
| 179 | - | ||
| 180 | - | // Show the window/tab | |
| 181 | - | gtk_widget_set_visible (GTK_WIDGET (window), TRUE); | |
| 182 | - | ||
| 183 | - | return TRUE; | |
| 184 | - | } | |
| 185 | - | ``` | |
| 186 | - | ||
| 187 | - | 4. **Close Handler**: Uses AdwTabView's built-in `close-page` signal (lines 3314-3360), which properly coordinates with WebKit | |
| 188 | - | ||
| 189 | - | **Key Lessons from Epiphany:** | |
| 190 | - | - ✅ Respects WebKit's signal-based lifecycle | |
| 191 | - | - ✅ Creates WebView synchronously in `create` handler | |
| 192 | - | - ✅ Returns WebView immediately to WebKit | |
| 193 | - | - ✅ Defers visibility to `ready-to-show` | |
| 194 | - | - ✅ Lets WebKit manage content loading | |
| 195 | - | - ✅ Clean separation: positioning logic vs. lifecycle management | |
| 196 | - | ||
| 197 | - | ### WebKitGTK Documentation | |
| 198 | - | ||
| 199 | - | **Reference:** https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.create.html | |
| 200 | - | ||
| 201 | - | **Signal: `WebKitWebView::create`** | |
| 202 | - | ||
| 203 | - | > This signal is emitted when a new WebKitWebView should be created for a popup window. The WebKitNavigationAction parameter contains information about the navigation action that triggered this signal. The signal handler should return the newly created WebKitWebView, or NULL if the popup window creation should be canceled. | |
| 204 | - | ||
| 205 | - | **Signal: `WebKitWebView::ready-to-show`** | |
| 206 | - | ||
| 207 | - | > Emitted after the WebKitWebView::create signal when the new view is ready to be shown. This signal allows you to show the new view and apply the window properties from webkit_web_view_get_window_properties() before the page is loaded. | |
| 208 | - | ||
| 209 | - | **Signal: `WebKitWebView::close`** | |
| 210 | - | ||
| 211 | - | > Emitted when the WebKitWebView is being closed by the web process. This signal is typically triggered by JavaScript window.close() calls. The signal handler should destroy the widget or otherwise handle the close request. | |
| 212 | - | ||
| 213 | - | ## Proposed Refactoring | |
| 214 | - | ||
| 215 | - | ### Phase 1: Implement Native WebKit Popup Signals | |
| 216 | - | ||
| 217 | - | #### 1.1 Add CGO Bindings for Create Signal | |
| 218 | - | ||
| 219 | - | **File:** `pkg/webkit/webview_popup.go` (new file) | |
| 220 | - | ||
| 221 | - | ```go | |
| 222 | - | package webkit | |
| 223 | - | ||
| 224 | - | /* | |
| 225 | - | #cgo pkg-config: webkitgtk-6.0 | |
| 226 | - | #include <webkit/webkit.h> | |
| 227 | - | #include <gtk/gtk.h> | |
| 228 | - | ||
| 229 | - | // Forward declaration | |
| 230 | - | extern WebKitWebView* goWebViewCreateCallback(WebKitWebView*, WebKitNavigationAction*, gpointer); | |
| 231 | - | ||
| 232 | - | // C helper to connect create signal | |
| 233 | - | static inline void connect_create_signal(WebKitWebView* view, gpointer user_data) { | |
| 234 | - | g_signal_connect(view, "create", G_CALLBACK(goWebViewCreateCallback), user_data); | |
| 235 | - | } | |
| 236 | - | */ | |
| 237 | - | import "C" | |
| 238 | - | import "unsafe" | |
| 239 | - | ||
| 240 | - | // ConnectCreateSignal connects the WebView's create signal for popup handling | |
| 241 | - | func (w *WebView) ConnectCreateSignal() { | |
| 242 | - | if w.view == nil { | |
| 243 | - | return | |
| 244 | - | } | |
| 245 | - | C.connect_create_signal((*C.WebKitWebView)(unsafe.Pointer(w.view.Native())), C.gpointer(unsafe.Pointer(w))) | |
| 246 | - | } | |
| 247 | - | ``` | |
| 248 | - | ||
| 249 | - | **File:** `pkg/webkit/webview_popup_callback.c` (new file) | |
| 250 | - | ||
| 251 | - | ```c | |
| 252 | - | #include <webkit/webkit.h> | |
| 253 | - | #include "_cgo_export.h" | |
| 254 | - | ||
| 255 | - | // CGO callback bridge for create signal | |
| 256 | - | WebKitWebView* goWebViewCreateCallback(WebKitWebView* parent_view, | |
| 257 | - | WebKitNavigationAction* navigation_action, | |
| 258 | - | gpointer user_data) { | |
| 259 | - | // Call Go callback via CGO | |
| 260 | - | return handleWebViewCreate(user_data, navigation_action); | |
| 261 | - | } | |
| 262 | - | ``` | |
| 263 | - | ||
| 264 | - | #### 1.2 Implement Go-side Create Handler | |
| 265 | - | ||
| 266 | - | **File:** `pkg/webkit/webview.go` (additions) | |
| 267 | - | ||
| 268 | - | ```go | |
| 269 | - | type WebView struct { | |
| 270 | - | // ... existing fields ... | |
| 271 | - | ||
| 272 | - | // Popup lifecycle handlers | |
| 273 | - | onPopupCreate func(*NavigationAction) *WebView | |
| 274 | - | onReadyToShow func() | |
| 275 | - | } | |
| 276 | - | ||
| 277 | - | //export handleWebViewCreate | |
| 278 | - | func handleWebViewCreate(webViewPtr unsafe.Pointer, navActionPtr unsafe.Pointer) *C.WebKitWebView { | |
| 279 | - | // Recover WebView from pointer | |
| 280 | - | w := (*WebView)(webViewPtr) | |
| 281 | - | ||
| 282 | - | if w.onPopupCreate == nil { | |
| 283 | - | return nil // No handler, cancel popup | |
| 284 | - | } | |
| 285 | - | ||
| 286 | - | // Parse navigation action | |
| 287 | - | navAction := parseNavigationAction(navActionPtr) | |
| 288 | - | ||
| 289 | - | // Call Go handler | |
| 290 | - | newWebView := w.onPopupCreate(navAction) | |
| 291 | - | ||
| 292 | - | if newWebView == nil { | |
| 293 | - | return nil | |
| 294 | - | } | |
| 295 | - | ||
| 296 | - | // Return WebKit's native view | |
| 297 | - | return (*C.WebKitWebView)(unsafe.Pointer(newWebView.view.Native())) | |
| 298 | - | } | |
| 299 | - | ||
| 300 | - | // RegisterPopupCreateHandler registers handler for WebKit's create signal | |
| 301 | - | func (w *WebView) RegisterPopupCreateHandler(handler func(*NavigationAction) *WebView) { | |
| 302 | - | w.mu.Lock() | |
| 303 | - | defer w.mu.Unlock() | |
| 304 | - | w.onPopupCreate = handler | |
| 305 | - | w.ConnectCreateSignal() | |
| 306 | - | } | |
| 307 | - | ``` | |
| 308 | - | ||
| 309 | - | #### 1.3 Connect Ready-to-Show and Close Signals | |
| 310 | - | ||
| 311 | - | **File:** `pkg/webkit/webview.go` (setupEventHandlers addition) | |
| 312 | - | ||
| 313 | - | ```go | |
| 314 | - | func (w *WebView) setupEventHandlers() { | |
| 315 | - | // ... existing handlers ... | |
| 316 | - | ||
| 317 | - | // Ready-to-show signal (already exists in gotk4 bindings) | |
| 318 | - | w.view.ConnectReadyToShow(func() { | |
| 319 | - | if w.onReadyToShow != nil { | |
| 320 | - | w.onReadyToShow() | |
| 321 | - | } | |
| 322 | - | }) | |
| 323 | - | ||
| 324 | - | // Close signal (already connected, line 219-223) | |
| 325 | - | // Just ensure onClose is used for popup lifecycle | |
| 326 | - | } | |
| 327 | - | ``` | |
| 328 | - | ||
| 329 | - | ### Phase 2: Refactor Workspace Popup Management | |
| 330 | - | ||
| 331 | - | #### 2.1 Remove JavaScript Interception for window.open | |
| 332 | - | ||
| 333 | - | **File:** `gui/src/lib/main-world/window-intercept.ts` | |
| 334 | - | ||
| 335 | - | **Change:** Remove `window.open` interception, let it reach WebKit natively | |
| 336 | - | ||
| 337 | - | **Rationale:** WebKit's `create` signal will handle it properly | |
| 338 | - | ||
| 339 | - | #### 2.2 Implement Workspace-Level Create Handler | |
| 340 | - | ||
| 341 | - | **File:** `internal/app/browser/workspace_popup.go` | |
| 342 | - | ||
| 343 | - | **Refactor:** Replace `handleIntentAsPopup()` with proper signal-based approach | |
| 344 | - | ||
| 345 | - | ```go | |
| 346 | - | // setupPopupHandling connects WebView to workspace popup management | |
| 347 | - | func (wm *WorkspaceManager) setupPopupHandling(webView *webkit.WebView, parentNode *paneNode) { | |
| 348 | - | webView.RegisterPopupCreateHandler(func(navAction *webkit.NavigationAction) *webkit.WebView { | |
| 349 | - | // Parse popup intent from navigation | |
| 350 | - | url := navAction.GetRequest().URI() | |
| 351 | - | features := navAction.GetWindowFeatures() | |
| 352 | - | ||
| 353 | - | // Determine popup type from config/URL | |
| 354 | - | popupType := wm.determinePopupType(url, features) | |
| 355 | - | ||
| 356 | - | // Create new WebView for popup | |
| 357 | - | popupView, err := wm.createWebViewFn() | |
| 358 | - | if err != nil { | |
| 359 | - | log.Printf("[workspace] Failed to create popup WebView: %v", err) | |
| 360 | - | return nil // Cancels popup | |
| 361 | - | } | |
| 362 | - | ||
| 363 | - | // Create pane but DON'T insert yet (WebKit still loading) | |
| 364 | - | popupPane, err := wm.createPaneFn(popupView) | |
| 365 | - | if err != nil { | |
| 366 | - | log.Printf("[workspace] Failed to create popup pane: %v", err) | |
| 367 | - | return nil | |
| 368 | - | } | |
| 369 | - | ||
| 370 | - | // Store popup info for ready-to-show | |
| 371 | - | popupInfo := &pendingPopup{ | |
| 372 | - | view: popupView, | |
| 373 | - | pane: popupPane, | |
| 374 | - | parentNode: parentNode, | |
| 375 | - | popupType: popupType, | |
| 376 | - | url: url, | |
| 377 | - | features: features, | |
| 378 | - | } | |
| 379 | - | wm.pendingPopups[popupView.ID()] = popupInfo | |
| 380 | - | ||
| 381 | - | // Connect ready-to-show for this specific popup | |
| 382 | - | popupView.RegisterReadyToShowHandler(func() { | |
| 383 | - | wm.handlePopupReadyToShow(popupView.ID()) | |
| 384 | - | }) | |
| 385 | - | ||
| 386 | - | // Connect close handler | |
| 387 | - | popupView.RegisterCloseHandler(func() { | |
| 388 | - | wm.handlePopupClose(popupView.ID()) | |
| 389 | - | }) | |
| 390 | - | ||
| 391 | - | // Return to WebKit - it will manage loading | |
| 392 | - | return popupView | |
| 393 | - | }) | |
| 394 | - | } | |
| 395 | - | ||
| 396 | - | // handlePopupReadyToShow inserts popup pane when WebKit says it's ready | |
| 397 | - | func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |
| 398 | - | info, ok := wm.pendingPopups[popupID] | |
| 399 | - | if !ok { | |
| 400 | - | return | |
| 401 | - | } | |
| 402 | - | delete(wm.pendingPopups, popupID) | |
| 403 | - | ||
| 404 | - | // NOW it's safe to insert into workspace | |
| 405 | - | switch info.popupType { | |
| 406 | - | case PopupTypeSplit: | |
| 407 | - | wm.insertPopupPane(info.parentNode, info.pane, "right") | |
| 408 | - | case PopupTypeStacked: | |
| 409 | - | wm.stackedPaneManager.StackPane(info.parentNode) | |
| 410 | - | case PopupTypeTabbed: | |
| 411 | - | // TODO: Implement tabbed popups | |
| 412 | - | wm.insertPopupPane(info.parentNode, info.pane, "right") | |
| 413 | - | } | |
| 414 | - | ||
| 415 | - | // Apply popup features (size, position, toolbar, etc.) | |
| 416 | - | wm.applyPopupFeatures(info.pane, info.features) | |
| 417 | - | } | |
| 418 | - | ||
| 419 | - | // handlePopupClose responds to WebKit's close signal | |
| 420 | - | func (wm *WorkspaceManager) handlePopupClose(popupID uint64) { | |
| 421 | - | // Find pane by WebView ID | |
| 422 | - | var targetNode *paneNode | |
| 423 | - | for webView, node := range wm.viewToNode { | |
| 424 | - | if webView != nil && webView.ID() == popupID { | |
| 425 | - | targetNode = node | |
| 426 | - | break | |
| 427 | - | } | |
| 428 | - | } | |
| 429 | - | ||
| 430 | - | if targetNode == nil { | |
| 431 | - | return | |
| 432 | - | } | |
| 433 | - | ||
| 434 | - | // Clean close via existing ClosePane (but WebKit already stopped) | |
| 435 | - | if err := wm.ClosePane(targetNode); err != nil { | |
| 436 | - | log.Printf("[workspace] Error closing popup pane: %v", err) | |
| 437 | - | } | |
| 438 | - | } | |
| 439 | - | ``` | |
| 440 | - | ||
| 441 | - | **Key Changes:** | |
| 442 | - | - ✅ WebView created in `create` signal handler | |
| 443 | - | - ✅ Returned immediately to WebKit | |
| 444 | - | - ✅ Pane insertion deferred until `ready-to-show` | |
| 445 | - | - ✅ Cleanup happens in response to `close` signal | |
| 446 | - | - ✅ No manual lifecycle management | |
| 447 | - | ||
| 448 | - | #### 2.3 Configuration-Based Popup Type | |
| 449 | - | ||
| 450 | - | **File:** `internal/app/config/config.go` | |
| 451 | - | ||
| 452 | - | ```go | |
| 453 | - | type PopupBehavior string | |
| 454 | - | ||
| 455 | - | const ( | |
| 456 | - | PopupBehaviorSplit PopupBehavior = "split" | |
| 457 | - | PopupBehaviorStacked PopupBehavior = "stacked" | |
| 458 | - | PopupBehaviorTabbed PopupBehavior = "tabbed" | |
| 459 | - | PopupBehaviorWindow PopupBehavior = "window" // Native popup window | |
| 460 | - | ) | |
| 461 | - | ||
| 462 | - | type Config struct { | |
| 463 | - | // ... existing fields ... | |
| 464 | - | ||
| 465 | - | PopupBehavior PopupBehavior `json:"popup_behavior"` | |
| 466 | - | ||
| 467 | - | // Per-domain popup overrides | |
| 468 | - | PopupOverrides map[string]PopupBehavior `json:"popup_overrides"` | |
| 469 | - | } | |
| 470 | - | ``` | |
| 471 | - | ||
| 472 | - | **Usage:** | |
| 473 | - | ```go | |
| 474 | - | func (wm *WorkspaceManager) determinePopupType(url string, features *webkit.WindowFeatures) PopupType { | |
| 475 | - | // Check per-domain overrides | |
| 476 | - | domain := extractDomain(url) | |
| 477 | - | if override, ok := wm.app.config.PopupOverrides[domain]; ok { | |
| 478 | - | return popupBehaviorToType(override) | |
| 479 | - | } | |
| 480 | - | ||
| 481 | - | // Use global config | |
| 482 | - | return popupBehaviorToType(wm.app.config.PopupBehavior) | |
| 483 | - | } | |
| 484 | - | ``` | |
| 485 | - | ||
| 486 | - | ### Phase 3: Remove Temporary Workarounds | |
| 487 | - | ||
| 488 | - | #### 3.1 Remove CSS Bloom Filter Workarounds | |
| 489 | - | ||
| 490 | - | **Files to clean:** | |
| 491 | - | - `internal/app/browser/workspace_types.go`: Remove `lastReparentTime`, `pendingHoverReattach`, `pendingFocusReattach` | |
| 492 | - | - `internal/app/browser/workspace_utils.go`: Remove timestamp checking in `applyActivePaneBorder()` | |
| 493 | - | - `internal/app/browser/workspace_pane_ops.go`: Remove timestamp setting in reparenting operations | |
| 494 | - | ||
| 495 | - | **Rationale:** Proper WebKit lifecycle management eliminates the race conditions | |
| 496 | - | ||
| 497 | - | #### 3.2 Remove LocalStorage Cleanup Workarounds | |
| 498 | - | ||
| 499 | - | **File:** `internal/app/browser/workspace_manager.go` | |
| 500 | - | ||
| 501 | - | **Remove:** All localStorage cleanup code in `close-popup` handler | |
| 502 | - | ||
| 503 | - | **Rationale:** | |
| 504 | - | - Popup's WebView will be destroyed cleanly by WebKit | |
| 505 | - | - Parent-child communication should use `window.postMessage` instead of localStorage | |
| 506 | - | - If localStorage is really needed, clean it in JavaScript before calling `window.close()` | |
| 507 | - | ||
| 508 | - | #### 3.3 Remove JavaScript Message-Based Popup Flow | |
| 509 | - | ||
| 510 | - | **Files:** | |
| 511 | - | - `gui/src/lib/main-world/window-intercept.ts`: Remove `window.open` interception | |
| 512 | - | - `internal/app/messaging/handler.go`: Remove `WindowIntent` and `create-pane` handling | |
| 513 | - | - `internal/app/messaging/deduplicator.go`: Potentially remove (may still be useful for other message types) | |
| 514 | - | ||
| 515 | - | ### Phase 4: Testing & Validation | |
| 516 | - | ||
| 517 | - | #### 4.1 Test Scenarios | |
| 518 | - | ||
| 519 | - | 1. **OAuth Flows:** | |
| 520 | - | - Google login | |
| 521 | - | - Notion login | |
| 522 | - | - GitHub OAuth | |
| 523 | - | ||
| 524 | - | 2. **Rapid Popup Cycles:** | |
| 525 | - | - Popup blocker detection | |
| 526 | - | - Sites that rapidly open/close popups | |
| 527 | - | ||
| 528 | - | 3. **Popup Features:** | |
| 529 | - | - Window.open with size specifications | |
| 530 | - | - Toolbar/menubar specifications | |
| 531 | - | - window.close() from JavaScript | |
| 532 | - | ||
| 533 | - | 4. **Configuration:** | |
| 534 | - | - Split popups | |
| 535 | - | - Stacked popups | |
| 536 | - | - Per-domain overrides | |
| 537 | - | ||
| 538 | - | #### 4.2 Success Criteria | |
| 539 | - | ||
| 540 | - | - ✅ Zero SIGSEGV crashes during popup lifecycle | |
| 541 | - | - ✅ No GTK bloom filter corruption warnings | |
| 542 | - | - ✅ OAuth flows complete successfully | |
| 543 | - | - ✅ Popups open in configured layout (split/stacked/tabbed) | |
| 544 | - | - ✅ Clean popup closure | |
| 545 | - | - ✅ Parent-child relationship respected | |
| 546 | - | - ✅ Window features honored | |
| 547 | - | ||
| 548 | - | ## Implementation Timeline | |
| 549 | - | ||
| 550 | - | ### Week 1: Foundation | |
| 551 | - | - [ ] Implement CGO bindings for create signal (`webview_popup.go`, `webview_popup_callback.c`) | |
| 552 | - | - [ ] Add Go-side create handler infrastructure | |
| 553 | - | - [ ] Test basic popup creation | |
| 554 | - | ||
| 555 | - | ### Week 2: Workspace Integration | |
| 556 | - | - [ ] Refactor workspace popup management | |
| 557 | - | - [ ] Implement `setupPopupHandling()`, `handlePopupReadyToShow()`, `handlePopupClose()` | |
| 558 | - | - [ ] Add popup type configuration | |
| 559 | - | - [ ] Test split/stacked popup layouts | |
| 560 | - | ||
| 561 | - | ### Week 3: Cleanup & Testing | |
| 562 | - | - [ ] Remove JavaScript interception | |
| 563 | - | - [ ] Remove temporary workarounds | |
| 564 | - | - [ ] Comprehensive testing of OAuth flows | |
| 565 | - | - [ ] Performance testing | |
| 566 | - | ||
| 567 | - | ### Week 4: Polish & Documentation | |
| 568 | - | - [ ] User documentation for popup configuration | |
| 569 | - | - [ ] Code comments and architecture documentation | |
| 570 | - | - [ ] Edge case testing | |
| 571 | - | - [ ] Production readiness review | |
| 572 | - | ||
| 573 | - | ## Migration Path | |
| 574 | - | ||
| 575 | - | ### Backward Compatibility | |
| 576 | - | ||
| 577 | - | During transition period, support both flows: | |
| 578 | - | ||
| 579 | - | ```go | |
| 580 | - | func (wm *WorkspaceManager) setupPopupHandling(webView *webkit.WebView, parentNode *paneNode) { | |
| 581 | - | if wm.app.config.UseNativePopupLifecycle { | |
| 582 | - | // New signal-based approach | |
| 583 | - | webView.RegisterPopupCreateHandler(...) | |
| 584 | - | } else { | |
| 585 | - | // Legacy JavaScript interception (deprecated) | |
| 586 | - | // Keep for one release cycle | |
| 587 | - | } | |
| 588 | - | } | |
| 589 | - | ``` | |
| 590 | - | ||
| 591 | - | Config flag: `"use_native_popup_lifecycle": true` | |
| 592 | - | ||
| 593 | - | ### Deprecation Notice | |
| 594 | - | ||
| 595 | - | Add to CHANGELOG: | |
| 596 | - | ``` | |
| 597 | - | ## v0.11.0 (Upcoming) | |
| 598 | - | ||
| 599 | - | ### BREAKING CHANGES | |
| 600 | - | - Popup window lifecycle now uses WebKit native signals | |
| 601 | - | - JavaScript window.open interception removed | |
| 602 | - | - Configuration changes: `popup_behavior` replaces internal logic | |
| 603 | - | ||
| 604 | - | ### Migration Guide | |
| 605 | - | - Update config.json with `popup_behavior` setting | |
| 606 | - | - Remove any custom window.open interception scripts | |
| 607 | - | - Test OAuth flows after upgrade | |
| 608 | - | ``` | |
| 609 | - | ||
| 610 | - | ## References | |
| 611 | - | ||
| 612 | - | ### WebKitGTK Documentation | |
| 613 | - | - Create Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.create.html | |
| 614 | - | - Ready-to-Show Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.ready-to-show.html | |
| 615 | - | - Close Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.close.html | |
| 616 | - | - WebKitWebView Class: `/usr/include/webkitgtk-6.0/webkit/WebKitWebView.h:243-259` | |
| 617 | - | - WebKitNavigationAction: https://webkitgtk.org/reference/webkitgtk/stable/class.NavigationAction.html | |
| 618 | - | - WebKitWindowProperties: https://webkitgtk.org/reference/webkitgtk/stable/class.WindowProperties.html | |
| 619 | - | ||
| 620 | - | ### GTK4 Documentation | |
| 621 | - | - GtkWidget Lifecycle: https://docs.gtk.org/gtk4/class.Widget.html | |
| 622 | - | - Signal Handling: https://docs.gtk.org/gobject/concepts.html#signals | |
| 623 | - | ||
| 624 | - | ### Reference Implementation | |
| 625 | - | - Epiphany Browser: `/home/brice/dev/clone/epiphany/src/ephy-window.c` | |
| 626 | - | - Create handler: lines 2034-2070 | |
| 627 | - | - Ready-to-show handler: lines 2012-2031 | |
| 628 | - | - Close handling: lines 3314-3360 | |
| 629 | - | ||
| 630 | - | ### Related Issues | |
| 631 | - | - Previous conversation summary: Session context showing attempted fixes | |
| 632 | - | - GTK Bloom Filter: https://blogs.gnome.org/otte/2012/12/26/css-hacker-news/ | |
| 633 | - | ||
| 634 | - | ## Risk Assessment | |
| 635 | - | ||
| 636 | - | ### High Risk | |
| 637 | - | - CGO callback stability | |
| 638 | - | - Thread safety in signal handlers | |
| 639 | - | - Potential gotk4 binding gaps | |
| 640 | - | ||
| 641 | - | ### Medium Risk | |
| 642 | - | - Configuration migration complexity | |
| 643 | - | - User workflow disruption during transition | |
| 644 | - | - Edge cases in popup features | |
| 645 | - | ||
| 646 | - | ### Low Risk | |
| 647 | - | - Performance impact (should improve) | |
| 648 | - | - Memory leaks (proper lifecycle should fix existing issues) | |
| 649 | - | ||
| 650 | - | ## Conclusion | |
| 651 | - | ||
| 652 | - | This refactoring aligns dumb-browser with WebKitGTK's architecture and proven patterns from Epiphany. By respecting WebKit's signal-based popup lifecycle, we eliminate race conditions and crashes while maintaining the unique split/stacked/tabbed popup positioning features. | |
| 653 | - | ||
| 654 | - | The key insight: **We control WHERE popups appear, WebKit controls WHEN and HOW they lifecycle.** | |
| @@ -1,654 +0,0 @@ | |||||
| 1 | - | # Popup Window Refactoring Plan | |||
| 2 | - | ||||
| 3 | - | ## Executive Summary | |||
| 4 | - | ||||
| 5 | - | The current popup window implementation causes critical SIGSEGV crashes due to improper lifecycle management that conflicts with WebKitGTK's internal architecture. This document outlines the issues, root causes, and a comprehensive refactoring plan to align with WebKitGTK's expected signal-based popup lifecycle. | |||
| 6 | - | ||||
| 7 | - | ## Issues Faced | |||
| 8 | - | ||||
| 9 | - | ### 1. Critical Crash: SIGSEGV on Popup Close | |||
| 10 | - | ||||
| 11 | - | **Symptom:** | |||
| 12 | - | ``` | |||
| 13 | - | signal 11 received but handler not on signal stack | |||
| 14 | - | fatal error: non-Go code set up signal handler without SA_ONSTACK flag | |||
| 15 | - | ``` | |||
| 16 | - | ||||
| 17 | - | **Occurrences:** | |||
| 18 | - | - OAuth popup completion (Google, Notion login) | |||
| 19 | - | - Rapid popup open/close cycles (popup blocker detection) | |||
| 20 | - | - Any programmatic popup closure | |||
| 21 | - | ||||
| 22 | - | **Impact:** Browser crash, data loss, poor user experience | |||
| 23 | - | ||||
| 24 | - | ### 2. GTK Bloom Filter Corruption | |||
| 25 | - | ||||
| 26 | - | **Symptom:** | |||
| 27 | - | ``` | |||
| 28 | - | [workspace] Skipping CSS, pane 0xc00015c000 reparented 0.6ms ago | |||
| 29 | - | ``` | |||
| 30 | - | ||||
| 31 | - | **Root Cause:** CSS class modifications during widget reparenting operations before GTK widget hierarchy stabilizes | |||
| 32 | - | ||||
| 33 | - | **Attempted Fixes (all failed):** | |||
| 34 | - | - Boolean flag `pendingReparent` cleared too early | |||
| 35 | - | - Timestamp-based CSS blocking (150ms safety window) | |||
| 36 | - | - `StopLoading()` before close (broke OAuth callback) | |||
| 37 | - | ||||
| 38 | - | ### 3. WebKit/GTK Race Conditions | |||
| 39 | - | ||||
| 40 | - | **Manifestations:** | |||
| 41 | - | - localStorage JS injection immediately before `ClosePane()` triggered WebKit JS engine during GTK reparenting | |||
| 42 | - | - Rapid popup opens (<1s) caused WebKit rendering pipeline initialization conflicts with GTK widget destruction | |||
| 43 | - | - WebKit signal handlers firing after GTK widgets destroyed | |||
| 44 | - | ||||
| 45 | - | ## Root Cause Analysis | |||
| 46 | - | ||||
| 47 | - | ### Architecture Violation: Bypassing WebKit Popup Lifecycle | |||
| 48 | - | ||||
| 49 | - | **Current Implementation Flow:** | |||
| 50 | - | ``` | |||
| 51 | - | JavaScript window.open() | |||
| 52 | - | ↓ | |||
| 53 | - | main-world.js intercepts | |||
| 54 | - | ↓ | |||
| 55 | - | postMessage to Go | |||
| 56 | - | ↓ | |||
| 57 | - | messaging.Handler | |||
| 58 | - | ↓ | |||
| 59 | - | WorkspaceManager.handleIntentAsPopup() | |||
| 60 | - | ↓ | |||
| 61 | - | Manual WebView creation | |||
| 62 | - | ↓ | |||
| 63 | - | Manual pane insertion | |||
| 64 | - | ↓ | |||
| 65 | - | Manual close (ClosePane) | |||
| 66 | - | ``` | |||
| 67 | - | ||||
| 68 | - | **Problem:** This completely bypasses WebKit's native popup management system. | |||
| 69 | - | ||||
| 70 | - | ### WebKitGTK's Expected Popup Lifecycle | |||
| 71 | - | ||||
| 72 | - | **Reference:** `/usr/include/webkitgtk-6.0/webkit/WebKitWebView.h:243-259` | |||
| 73 | - | ||||
| 74 | - | ```c | |||
| 75 | - | struct _WebKitWebViewClass { | |||
| 76 | - | // ... other signals ... | |||
| 77 | - | ||||
| 78 | - | GtkWidget *(* create) (WebKitWebView *web_view, | |||
| 79 | - | WebKitNavigationAction *navigation_action); | |||
| 80 | - | ||||
| 81 | - | void (* ready_to_show) (WebKitWebView *web_view); | |||
| 82 | - | ||||
| 83 | - | void (* close) (WebKitWebView *web_view); | |||
| 84 | - | ||||
| 85 | - | // ... other signals ... | |||
| 86 | - | }; | |||
| 87 | - | ``` | |||
| 88 | - | ||||
| 89 | - | **WebKit's Lifecycle Contract:** | |||
| 90 | - | ||||
| 91 | - | 1. **`create` signal**: WebKit requests new WebView for popup | |||
| 92 | - | - Application creates and configures new WebView | |||
| 93 | - | - Application returns WebView to WebKit | |||
| 94 | - | - **WebKit takes ownership of loading and content rendering** | |||
| 95 | - | ||||
| 96 | - | 2. **`ready-to-show` signal**: WebKit indicates popup is ready | |||
| 97 | - | - WebKit has finished initial setup | |||
| 98 | - | - Safe to show/position window/pane | |||
| 99 | - | - Window properties (size, features) are available | |||
| 100 | - | ||||
| 101 | - | 3. **`close` signal**: WebKit requests popup closure | |||
| 102 | - | - Emitted when JavaScript calls `window.close()` | |||
| 103 | - | - Emitted when page navigates away from popup context | |||
| 104 | - | - **Application should destroy widgets in response** | |||
| 105 | - | ||||
| 106 | - | ### Why Current Implementation Fails | |||
| 107 | - | ||||
| 108 | - | 1. **No signal connection**: Application never connects to WebKit's `create` signal | |||
| 109 | - | 2. **Manual lifecycle**: Application creates WebViews independently, breaking WebKit's internal reference tracking | |||
| 110 | - | 3. **Premature destruction**: `ClosePane()` destroys GTK widgets while WebKit still has: | |||
| 111 | - | - Active signal handlers | |||
| 112 | - | - Compositor threads running | |||
| 113 | - | - Rendering pipeline active | |||
| 114 | - | - JS engine references | |||
| 115 | - | ||||
| 116 | - | 4. **Race conditions**: JavaScript message handling races with WebKit's internal operations | |||
| 117 | - | ||||
| 118 | - | ## Reference Implementations | |||
| 119 | - | ||||
| 120 | - | ### Epiphany (GNOME Web Browser) | |||
| 121 | - | ||||
| 122 | - | **File:** `/home/brice/dev/clone/epiphany/src/ephy-window.c` | |||
| 123 | - | ||||
| 124 | - | **Key Implementation Points:** | |||
| 125 | - | ||||
| 126 | - | 1. **Signal Connection** (line 2636-2638): | |||
| 127 | - | ```c | |||
| 128 | - | g_signal_connect_object (web_view, "create", | |||
| 129 | - | G_CALLBACK (create_web_view_cb), | |||
| 130 | - | window, 0); | |||
| 131 | - | ``` | |||
| 132 | - | ||||
| 133 | - | 2. **Create Handler** (lines 2034-2070): | |||
| 134 | - | ```c | |||
| 135 | - | static WebKitWebView * | |||
| 136 | - | create_web_view_cb (WebKitWebView *web_view, | |||
| 137 | - | WebKitNavigationAction *navigation_action, | |||
| 138 | - | EphyWindow *window) | |||
| 139 | - | { | |||
| 140 | - | EphyEmbed *embed; | |||
| 141 | - | WebKitWebView *new_web_view; | |||
| 142 | - | ||||
| 143 | - | // Determine if popup should be new window or new tab | |||
| 144 | - | if (new_windows_in_tabs_setting) { | |||
| 145 | - | target_window = window; // Same window (tab) | |||
| 146 | - | } else { | |||
| 147 | - | target_window = ephy_window_new (); // New window | |||
| 148 | - | } | |||
| 149 | - | ||||
| 150 | - | // Create new tab/window with parent relationship | |||
| 151 | - | embed = ephy_shell_new_tab_full (..., web_view, ...); | |||
| 152 | - | ||||
| 153 | - | new_web_view = EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED (embed); | |||
| 154 | - | ||||
| 155 | - | // Connect ready-to-show handler | |||
| 156 | - | g_signal_connect_object (new_web_view, "ready-to-show", | |||
| 157 | - | G_CALLBACK (web_view_ready_cb), | |||
| 158 | - | web_view, 0); | |||
| 159 | - | ||||
| 160 | - | // Return new WebView to WebKit | |||
| 161 | - | return new_web_view; | |||
| 162 | - | } | |||
| 163 | - | ``` | |||
| 164 | - | ||||
| 165 | - | 3. **Ready-to-Show Handler** (lines 2012-2031): | |||
| 166 | - | ```c | |||
| 167 | - | static gboolean | |||
| 168 | - | web_view_ready_cb (WebKitWebView *web_view, | |||
| 169 | - | WebKitWebView *parent_web_view) | |||
| 170 | - | { | |||
| 171 | - | EphyWindow *window; | |||
| 172 | - | ||||
| 173 | - | window = EPHY_WINDOW (gtk_widget_get_root (GTK_WIDGET (web_view))); | |||
| 174 | - | ||||
| 175 | - | // Apply window properties | |||
| 176 | - | if (using_new_window) { | |||
| 177 | - | ephy_window_configure_for_view (window, web_view); | |||
| 178 | - | } | |||
| 179 | - | ||||
| 180 | - | // Show the window/tab | |||
| 181 | - | gtk_widget_set_visible (GTK_WIDGET (window), TRUE); | |||
| 182 | - | ||||
| 183 | - | return TRUE; | |||
| 184 | - | } | |||
| 185 | - | ``` | |||
| 186 | - | ||||
| 187 | - | 4. **Close Handler**: Uses AdwTabView's built-in `close-page` signal (lines 3314-3360), which properly coordinates with WebKit | |||
| 188 | - | ||||
| 189 | - | **Key Lessons from Epiphany:** | |||
| 190 | - | - ✅ Respects WebKit's signal-based lifecycle | |||
| 191 | - | - ✅ Creates WebView synchronously in `create` handler | |||
| 192 | - | - ✅ Returns WebView immediately to WebKit | |||
| 193 | - | - ✅ Defers visibility to `ready-to-show` | |||
| 194 | - | - ✅ Lets WebKit manage content loading | |||
| 195 | - | - ✅ Clean separation: positioning logic vs. lifecycle management | |||
| 196 | - | ||||
| 197 | - | ### WebKitGTK Documentation | |||
| 198 | - | ||||
| 199 | - | **Reference:** https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.create.html | |||
| 200 | - | ||||
| 201 | - | **Signal: `WebKitWebView::create`** | |||
| 202 | - | ||||
| 203 | - | > This signal is emitted when a new WebKitWebView should be created for a popup window. The WebKitNavigationAction parameter contains information about the navigation action that triggered this signal. The signal handler should return the newly created WebKitWebView, or NULL if the popup window creation should be canceled. | |||
| 204 | - | ||||
| 205 | - | **Signal: `WebKitWebView::ready-to-show`** | |||
| 206 | - | ||||
| 207 | - | > Emitted after the WebKitWebView::create signal when the new view is ready to be shown. This signal allows you to show the new view and apply the window properties from webkit_web_view_get_window_properties() before the page is loaded. | |||
| 208 | - | ||||
| 209 | - | **Signal: `WebKitWebView::close`** | |||
| 210 | - | ||||
| 211 | - | > Emitted when the WebKitWebView is being closed by the web process. This signal is typically triggered by JavaScript window.close() calls. The signal handler should destroy the widget or otherwise handle the close request. | |||
| 212 | - | ||||
| 213 | - | ## Proposed Refactoring | |||
| 214 | - | ||||
| 215 | - | ### Phase 1: Implement Native WebKit Popup Signals | |||
| 216 | - | ||||
| 217 | - | #### 1.1 Add CGO Bindings for Create Signal | |||
| 218 | - | ||||
| 219 | - | **File:** `pkg/webkit/webview_popup.go` (new file) | |||
| 220 | - | ||||
| 221 | - | ```go | |||
| 222 | - | package webkit | |||
| 223 | - | ||||
| 224 | - | /* | |||
| 225 | - | #cgo pkg-config: webkitgtk-6.0 | |||
| 226 | - | #include <webkit/webkit.h> | |||
| 227 | - | #include <gtk/gtk.h> | |||
| 228 | - | ||||
| 229 | - | // Forward declaration | |||
| 230 | - | extern WebKitWebView* goWebViewCreateCallback(WebKitWebView*, WebKitNavigationAction*, gpointer); | |||
| 231 | - | ||||
| 232 | - | // C helper to connect create signal | |||
| 233 | - | static inline void connect_create_signal(WebKitWebView* view, gpointer user_data) { | |||
| 234 | - | g_signal_connect(view, "create", G_CALLBACK(goWebViewCreateCallback), user_data); | |||
| 235 | - | } | |||
| 236 | - | */ | |||
| 237 | - | import "C" | |||
| 238 | - | import "unsafe" | |||
| 239 | - | ||||
| 240 | - | // ConnectCreateSignal connects the WebView's create signal for popup handling | |||
| 241 | - | func (w *WebView) ConnectCreateSignal() { | |||
| 242 | - | if w.view == nil { | |||
| 243 | - | return | |||
| 244 | - | } | |||
| 245 | - | C.connect_create_signal((*C.WebKitWebView)(unsafe.Pointer(w.view.Native())), C.gpointer(unsafe.Pointer(w))) | |||
| 246 | - | } | |||
| 247 | - | ``` | |||
| 248 | - | ||||
| 249 | - | **File:** `pkg/webkit/webview_popup_callback.c` (new file) | |||
| 250 | - | ||||
| 251 | - | ```c | |||
| 252 | - | #include <webkit/webkit.h> | |||
| 253 | - | #include "_cgo_export.h" | |||
| 254 | - | ||||
| 255 | - | // CGO callback bridge for create signal | |||
| 256 | - | WebKitWebView* goWebViewCreateCallback(WebKitWebView* parent_view, | |||
| 257 | - | WebKitNavigationAction* navigation_action, | |||
| 258 | - | gpointer user_data) { | |||
| 259 | - | // Call Go callback via CGO | |||
| 260 | - | return handleWebViewCreate(user_data, navigation_action); | |||
| 261 | - | } | |||
| 262 | - | ``` | |||
| 263 | - | ||||
| 264 | - | #### 1.2 Implement Go-side Create Handler | |||
| 265 | - | ||||
| 266 | - | **File:** `pkg/webkit/webview.go` (additions) | |||
| 267 | - | ||||
| 268 | - | ```go | |||
| 269 | - | type WebView struct { | |||
| 270 | - | // ... existing fields ... | |||
| 271 | - | ||||
| 272 | - | // Popup lifecycle handlers | |||
| 273 | - | onPopupCreate func(*NavigationAction) *WebView | |||
| 274 | - | onReadyToShow func() | |||
| 275 | - | } | |||
| 276 | - | ||||
| 277 | - | //export handleWebViewCreate | |||
| 278 | - | func handleWebViewCreate(webViewPtr unsafe.Pointer, navActionPtr unsafe.Pointer) *C.WebKitWebView { | |||
| 279 | - | // Recover WebView from pointer | |||
| 280 | - | w := (*WebView)(webViewPtr) | |||
| 281 | - | ||||
| 282 | - | if w.onPopupCreate == nil { | |||
| 283 | - | return nil // No handler, cancel popup | |||
| 284 | - | } | |||
| 285 | - | ||||
| 286 | - | // Parse navigation action | |||
| 287 | - | navAction := parseNavigationAction(navActionPtr) | |||
| 288 | - | ||||
| 289 | - | // Call Go handler | |||
| 290 | - | newWebView := w.onPopupCreate(navAction) | |||
| 291 | - | ||||
| 292 | - | if newWebView == nil { | |||
| 293 | - | return nil | |||
| 294 | - | } | |||
| 295 | - | ||||
| 296 | - | // Return WebKit's native view | |||
| 297 | - | return (*C.WebKitWebView)(unsafe.Pointer(newWebView.view.Native())) | |||
| 298 | - | } | |||
| 299 | - | ||||
| 300 | - | // RegisterPopupCreateHandler registers handler for WebKit's create signal | |||
| 301 | - | func (w *WebView) RegisterPopupCreateHandler(handler func(*NavigationAction) *WebView) { | |||
| 302 | - | w.mu.Lock() | |||
| 303 | - | defer w.mu.Unlock() | |||
| 304 | - | w.onPopupCreate = handler | |||
| 305 | - | w.ConnectCreateSignal() | |||
| 306 | - | } | |||
| 307 | - | ``` | |||
| 308 | - | ||||
| 309 | - | #### 1.3 Connect Ready-to-Show and Close Signals | |||
| 310 | - | ||||
| 311 | - | **File:** `pkg/webkit/webview.go` (setupEventHandlers addition) | |||
| 312 | - | ||||
| 313 | - | ```go | |||
| 314 | - | func (w *WebView) setupEventHandlers() { | |||
| 315 | - | // ... existing handlers ... | |||
| 316 | - | ||||
| 317 | - | // Ready-to-show signal (already exists in gotk4 bindings) | |||
| 318 | - | w.view.ConnectReadyToShow(func() { | |||
| 319 | - | if w.onReadyToShow != nil { | |||
| 320 | - | w.onReadyToShow() | |||
| 321 | - | } | |||
| 322 | - | }) | |||
| 323 | - | ||||
| 324 | - | // Close signal (already connected, line 219-223) | |||
| 325 | - | // Just ensure onClose is used for popup lifecycle | |||
| 326 | - | } | |||
| 327 | - | ``` | |||
| 328 | - | ||||
| 329 | - | ### Phase 2: Refactor Workspace Popup Management | |||
| 330 | - | ||||
| 331 | - | #### 2.1 Remove JavaScript Interception for window.open | |||
| 332 | - | ||||
| 333 | - | **File:** `gui/src/lib/main-world/window-intercept.ts` | |||
| 334 | - | ||||
| 335 | - | **Change:** Remove `window.open` interception, let it reach WebKit natively | |||
| 336 | - | ||||
| 337 | - | **Rationale:** WebKit's `create` signal will handle it properly | |||
| 338 | - | ||||
| 339 | - | #### 2.2 Implement Workspace-Level Create Handler | |||
| 340 | - | ||||
| 341 | - | **File:** `internal/app/browser/workspace_popup.go` | |||
| 342 | - | ||||
| 343 | - | **Refactor:** Replace `handleIntentAsPopup()` with proper signal-based approach | |||
| 344 | - | ||||
| 345 | - | ```go | |||
| 346 | - | // setupPopupHandling connects WebView to workspace popup management | |||
| 347 | - | func (wm *WorkspaceManager) setupPopupHandling(webView *webkit.WebView, parentNode *paneNode) { | |||
| 348 | - | webView.RegisterPopupCreateHandler(func(navAction *webkit.NavigationAction) *webkit.WebView { | |||
| 349 | - | // Parse popup intent from navigation | |||
| 350 | - | url := navAction.GetRequest().URI() | |||
| 351 | - | features := navAction.GetWindowFeatures() | |||
| 352 | - | ||||
| 353 | - | // Determine popup type from config/URL | |||
| 354 | - | popupType := wm.determinePopupType(url, features) | |||
| 355 | - | ||||
| 356 | - | // Create new WebView for popup | |||
| 357 | - | popupView, err := wm.createWebViewFn() | |||
| 358 | - | if err != nil { | |||
| 359 | - | log.Printf("[workspace] Failed to create popup WebView: %v", err) | |||
| 360 | - | return nil // Cancels popup | |||
| 361 | - | } | |||
| 362 | - | ||||
| 363 | - | // Create pane but DON'T insert yet (WebKit still loading) | |||
| 364 | - | popupPane, err := wm.createPaneFn(popupView) | |||
| 365 | - | if err != nil { | |||
| 366 | - | log.Printf("[workspace] Failed to create popup pane: %v", err) | |||
| 367 | - | return nil | |||
| 368 | - | } | |||
| 369 | - | ||||
| 370 | - | // Store popup info for ready-to-show | |||
| 371 | - | popupInfo := &pendingPopup{ | |||
| 372 | - | view: popupView, | |||
| 373 | - | pane: popupPane, | |||
| 374 | - | parentNode: parentNode, | |||
| 375 | - | popupType: popupType, | |||
| 376 | - | url: url, | |||
| 377 | - | features: features, | |||
| 378 | - | } | |||
| 379 | - | wm.pendingPopups[popupView.ID()] = popupInfo | |||
| 380 | - | ||||
| 381 | - | // Connect ready-to-show for this specific popup | |||
| 382 | - | popupView.RegisterReadyToShowHandler(func() { | |||
| 383 | - | wm.handlePopupReadyToShow(popupView.ID()) | |||
| 384 | - | }) | |||
| 385 | - | ||||
| 386 | - | // Connect close handler | |||
| 387 | - | popupView.RegisterCloseHandler(func() { | |||
| 388 | - | wm.handlePopupClose(popupView.ID()) | |||
| 389 | - | }) | |||
| 390 | - | ||||
| 391 | - | // Return to WebKit - it will manage loading | |||
| 392 | - | return popupView | |||
| 393 | - | }) | |||
| 394 | - | } | |||
| 395 | - | ||||
| 396 | - | // handlePopupReadyToShow inserts popup pane when WebKit says it's ready | |||
| 397 | - | func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 398 | - | info, ok := wm.pendingPopups[popupID] | |||
| 399 | - | if !ok { | |||
| 400 | - | return | |||
| 401 | - | } | |||
| 402 | - | delete(wm.pendingPopups, popupID) | |||
| 403 | - | ||||
| 404 | - | // NOW it's safe to insert into workspace | |||
| 405 | - | switch info.popupType { | |||
| 406 | - | case PopupTypeSplit: | |||
| 407 | - | wm.insertPopupPane(info.parentNode, info.pane, "right") | |||
| 408 | - | case PopupTypeStacked: | |||
| 409 | - | wm.stackedPaneManager.StackPane(info.parentNode) | |||
| 410 | - | case PopupTypeTabbed: | |||
| 411 | - | // TODO: Implement tabbed popups | |||
| 412 | - | wm.insertPopupPane(info.parentNode, info.pane, "right") | |||
| 413 | - | } | |||
| 414 | - | ||||
| 415 | - | // Apply popup features (size, position, toolbar, etc.) | |||
| 416 | - | wm.applyPopupFeatures(info.pane, info.features) | |||
| 417 | - | } | |||
| 418 | - | ||||
| 419 | - | // handlePopupClose responds to WebKit's close signal | |||
| 420 | - | func (wm *WorkspaceManager) handlePopupClose(popupID uint64) { | |||
| 421 | - | // Find pane by WebView ID | |||
| 422 | - | var targetNode *paneNode | |||
| 423 | - | for webView, node := range wm.viewToNode { | |||
| 424 | - | if webView != nil && webView.ID() == popupID { | |||
| 425 | - | targetNode = node | |||
| 426 | - | break | |||
| 427 | - | } | |||
| 428 | - | } | |||
| 429 | - | ||||
| 430 | - | if targetNode == nil { | |||
| 431 | - | return | |||
| 432 | - | } | |||
| 433 | - | ||||
| 434 | - | // Clean close via existing ClosePane (but WebKit already stopped) | |||
| 435 | - | if err := wm.ClosePane(targetNode); err != nil { | |||
| 436 | - | log.Printf("[workspace] Error closing popup pane: %v", err) | |||
| 437 | - | } | |||
| 438 | - | } | |||
| 439 | - | ``` | |||
| 440 | - | ||||
| 441 | - | **Key Changes:** | |||
| 442 | - | - ✅ WebView created in `create` signal handler | |||
| 443 | - | - ✅ Returned immediately to WebKit | |||
| 444 | - | - ✅ Pane insertion deferred until `ready-to-show` | |||
| 445 | - | - ✅ Cleanup happens in response to `close` signal | |||
| 446 | - | - ✅ No manual lifecycle management | |||
| 447 | - | ||||
| 448 | - | #### 2.3 Configuration-Based Popup Type | |||
| 449 | - | ||||
| 450 | - | **File:** `internal/app/config/config.go` | |||
| 451 | - | ||||
| 452 | - | ```go | |||
| 453 | - | type PopupBehavior string | |||
| 454 | - | ||||
| 455 | - | const ( | |||
| 456 | - | PopupBehaviorSplit PopupBehavior = "split" | |||
| 457 | - | PopupBehaviorStacked PopupBehavior = "stacked" | |||
| 458 | - | PopupBehaviorTabbed PopupBehavior = "tabbed" | |||
| 459 | - | PopupBehaviorWindow PopupBehavior = "window" // Native popup window | |||
| 460 | - | ) | |||
| 461 | - | ||||
| 462 | - | type Config struct { | |||
| 463 | - | // ... existing fields ... | |||
| 464 | - | ||||
| 465 | - | PopupBehavior PopupBehavior `json:"popup_behavior"` | |||
| 466 | - | ||||
| 467 | - | // Per-domain popup overrides | |||
| 468 | - | PopupOverrides map[string]PopupBehavior `json:"popup_overrides"` | |||
| 469 | - | } | |||
| 470 | - | ``` | |||
| 471 | - | ||||
| 472 | - | **Usage:** | |||
| 473 | - | ```go | |||
| 474 | - | func (wm *WorkspaceManager) determinePopupType(url string, features *webkit.WindowFeatures) PopupType { | |||
| 475 | - | // Check per-domain overrides | |||
| 476 | - | domain := extractDomain(url) | |||
| 477 | - | if override, ok := wm.app.config.PopupOverrides[domain]; ok { | |||
| 478 | - | return popupBehaviorToType(override) | |||
| 479 | - | } | |||
| 480 | - | ||||
| 481 | - | // Use global config | |||
| 482 | - | return popupBehaviorToType(wm.app.config.PopupBehavior) | |||
| 483 | - | } | |||
| 484 | - | ``` | |||
| 485 | - | ||||
| 486 | - | ### Phase 3: Remove Temporary Workarounds | |||
| 487 | - | ||||
| 488 | - | #### 3.1 Remove CSS Bloom Filter Workarounds | |||
| 489 | - | ||||
| 490 | - | **Files to clean:** | |||
| 491 | - | - `internal/app/browser/workspace_types.go`: Remove `lastReparentTime`, `pendingHoverReattach`, `pendingFocusReattach` | |||
| 492 | - | - `internal/app/browser/workspace_utils.go`: Remove timestamp checking in `applyActivePaneBorder()` | |||
| 493 | - | - `internal/app/browser/workspace_pane_ops.go`: Remove timestamp setting in reparenting operations | |||
| 494 | - | ||||
| 495 | - | **Rationale:** Proper WebKit lifecycle management eliminates the race conditions | |||
| 496 | - | ||||
| 497 | - | #### 3.2 Remove LocalStorage Cleanup Workarounds | |||
| 498 | - | ||||
| 499 | - | **File:** `internal/app/browser/workspace_manager.go` | |||
| 500 | - | ||||
| 501 | - | **Remove:** All localStorage cleanup code in `close-popup` handler | |||
| 502 | - | ||||
| 503 | - | **Rationale:** | |||
| 504 | - | - Popup's WebView will be destroyed cleanly by WebKit | |||
| 505 | - | - Parent-child communication should use `window.postMessage` instead of localStorage | |||
| 506 | - | - If localStorage is really needed, clean it in JavaScript before calling `window.close()` | |||
| 507 | - | ||||
| 508 | - | #### 3.3 Remove JavaScript Message-Based Popup Flow | |||
| 509 | - | ||||
| 510 | - | **Files:** | |||
| 511 | - | - `gui/src/lib/main-world/window-intercept.ts`: Remove `window.open` interception | |||
| 512 | - | - `internal/app/messaging/handler.go`: Remove `WindowIntent` and `create-pane` handling | |||
| 513 | - | - `internal/app/messaging/deduplicator.go`: Potentially remove (may still be useful for other message types) | |||
| 514 | - | ||||
| 515 | - | ### Phase 4: Testing & Validation | |||
| 516 | - | ||||
| 517 | - | #### 4.1 Test Scenarios | |||
| 518 | - | ||||
| 519 | - | 1. **OAuth Flows:** | |||
| 520 | - | - Google login | |||
| 521 | - | - Notion login | |||
| 522 | - | - GitHub OAuth | |||
| 523 | - | ||||
| 524 | - | 2. **Rapid Popup Cycles:** | |||
| 525 | - | - Popup blocker detection | |||
| 526 | - | - Sites that rapidly open/close popups | |||
| 527 | - | ||||
| 528 | - | 3. **Popup Features:** | |||
| 529 | - | - Window.open with size specifications | |||
| 530 | - | - Toolbar/menubar specifications | |||
| 531 | - | - window.close() from JavaScript | |||
| 532 | - | ||||
| 533 | - | 4. **Configuration:** | |||
| 534 | - | - Split popups | |||
| 535 | - | - Stacked popups | |||
| 536 | - | - Per-domain overrides | |||
| 537 | - | ||||
| 538 | - | #### 4.2 Success Criteria | |||
| 539 | - | ||||
| 540 | - | - ✅ Zero SIGSEGV crashes during popup lifecycle | |||
| 541 | - | - ✅ No GTK bloom filter corruption warnings | |||
| 542 | - | - ✅ OAuth flows complete successfully | |||
| 543 | - | - ✅ Popups open in configured layout (split/stacked/tabbed) | |||
| 544 | - | - ✅ Clean popup closure | |||
| 545 | - | - ✅ Parent-child relationship respected | |||
| 546 | - | - ✅ Window features honored | |||
| 547 | - | ||||
| 548 | - | ## Implementation Timeline | |||
| 549 | - | ||||
| 550 | - | ### Week 1: Foundation | |||
| 551 | - | - [ ] Implement CGO bindings for create signal (`webview_popup.go`, `webview_popup_callback.c`) | |||
| 552 | - | - [ ] Add Go-side create handler infrastructure | |||
| 553 | - | - [ ] Test basic popup creation | |||
| 554 | - | ||||
| 555 | - | ### Week 2: Workspace Integration | |||
| 556 | - | - [ ] Refactor workspace popup management | |||
| 557 | - | - [ ] Implement `setupPopupHandling()`, `handlePopupReadyToShow()`, `handlePopupClose()` | |||
| 558 | - | - [ ] Add popup type configuration | |||
| 559 | - | - [ ] Test split/stacked popup layouts | |||
| 560 | - | ||||
| 561 | - | ### Week 3: Cleanup & Testing | |||
| 562 | - | - [ ] Remove JavaScript interception | |||
| 563 | - | - [ ] Remove temporary workarounds | |||
| 564 | - | - [ ] Comprehensive testing of OAuth flows | |||
| 565 | - | - [ ] Performance testing | |||
| 566 | - | ||||
| 567 | - | ### Week 4: Polish & Documentation | |||
| 568 | - | - [ ] User documentation for popup configuration | |||
| 569 | - | - [ ] Code comments and architecture documentation | |||
| 570 | - | - [ ] Edge case testing | |||
| 571 | - | - [ ] Production readiness review | |||
| 572 | - | ||||
| 573 | - | ## Migration Path | |||
| 574 | - | ||||
| 575 | - | ### Backward Compatibility | |||
| 576 | - | ||||
| 577 | - | During transition period, support both flows: | |||
| 578 | - | ||||
| 579 | - | ```go | |||
| 580 | - | func (wm *WorkspaceManager) setupPopupHandling(webView *webkit.WebView, parentNode *paneNode) { | |||
| 581 | - | if wm.app.config.UseNativePopupLifecycle { | |||
| 582 | - | // New signal-based approach | |||
| 583 | - | webView.RegisterPopupCreateHandler(...) | |||
| 584 | - | } else { | |||
| 585 | - | // Legacy JavaScript interception (deprecated) | |||
| 586 | - | // Keep for one release cycle | |||
| 587 | - | } | |||
| 588 | - | } | |||
| 589 | - | ``` | |||
| 590 | - | ||||
| 591 | - | Config flag: `"use_native_popup_lifecycle": true` | |||
| 592 | - | ||||
| 593 | - | ### Deprecation Notice | |||
| 594 | - | ||||
| 595 | - | Add to CHANGELOG: | |||
| 596 | - | ``` | |||
| 597 | - | ## v0.11.0 (Upcoming) | |||
| 598 | - | ||||
| 599 | - | ### BREAKING CHANGES | |||
| 600 | - | - Popup window lifecycle now uses WebKit native signals | |||
| 601 | - | - JavaScript window.open interception removed | |||
| 602 | - | - Configuration changes: `popup_behavior` replaces internal logic | |||
| 603 | - | ||||
| 604 | - | ### Migration Guide | |||
| 605 | - | - Update config.json with `popup_behavior` setting | |||
| 606 | - | - Remove any custom window.open interception scripts | |||
| 607 | - | - Test OAuth flows after upgrade | |||
| 608 | - | ``` | |||
| 609 | - | ||||
| 610 | - | ## References | |||
| 611 | - | ||||
| 612 | - | ### WebKitGTK Documentation | |||
| 613 | - | - Create Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.create.html | |||
| 614 | - | - Ready-to-Show Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.ready-to-show.html | |||
| 615 | - | - Close Signal: https://webkitgtk.org/reference/webkitgtk/stable/signal.WebView.close.html | |||
| 616 | - | - WebKitWebView Class: `/usr/include/webkitgtk-6.0/webkit/WebKitWebView.h:243-259` | |||
| 617 | - | - WebKitNavigationAction: https://webkitgtk.org/reference/webkitgtk/stable/class.NavigationAction.html | |||
| 618 | - | - WebKitWindowProperties: https://webkitgtk.org/reference/webkitgtk/stable/class.WindowProperties.html | |||
| 619 | - | ||||
| 620 | - | ### GTK4 Documentation | |||
| 621 | - | - GtkWidget Lifecycle: https://docs.gtk.org/gtk4/class.Widget.html | |||
| 622 | - | - Signal Handling: https://docs.gtk.org/gobject/concepts.html#signals | |||
| 623 | - | ||||
| 624 | - | ### Reference Implementation | |||
| 625 | - | - Epiphany Browser: `/home/brice/dev/clone/epiphany/src/ephy-window.c` | |||
| 626 | - | - Create handler: lines 2034-2070 | |||
| 627 | - | - Ready-to-show handler: lines 2012-2031 | |||
| 628 | - | - Close handling: lines 3314-3360 | |||
| 629 | - | ||||
| 630 | - | ### Related Issues | |||
| 631 | - | - Previous conversation summary: Session context showing attempted fixes | |||
| 632 | - | - GTK Bloom Filter: https://blogs.gnome.org/otte/2012/12/26/css-hacker-news/ | |||
| 633 | - | ||||
| 634 | - | ## Risk Assessment | |||
| 635 | - | ||||
| 636 | - | ### High Risk | |||
| 637 | - | - CGO callback stability | |||
| 638 | - | - Thread safety in signal handlers | |||
| 639 | - | - Potential gotk4 binding gaps | |||
| 640 | - | ||||
| 641 | - | ### Medium Risk | |||
| 642 | - | - Configuration migration complexity | |||
| 643 | - | - User workflow disruption during transition | |||
| 644 | - | - Edge cases in popup features | |||
| 645 | - | ||||
| 646 | - | ### Low Risk | |||
| 647 | - | - Performance impact (should improve) | |||
| 648 | - | - Memory leaks (proper lifecycle should fix existing issues) | |||
| 649 | - | ||||
| 650 | - | ## Conclusion | |||
| 651 | - | ||||
| 652 | - | This refactoring aligns dumb-browser with WebKitGTK's architecture and proven patterns from Epiphany. By respecting WebKit's signal-based popup lifecycle, we eliminate race conditions and crashes while maintaining the unique split/stacked/tabbed popup positioning features. | |||
| 653 | - | ||||
| 654 | - | The key insight: **We control WHERE popups appear, WebKit controls WHEN and HOW they lifecycle.** | |||
M README.md +1 -1
| @@ -368,7 +368,7 @@ WebKitGTK uses GStreamer for media playback. Dumber includes automatic hardware | |||
| 368 | 368 | **Required packages:** | |
| 369 | 369 | - Arch Linux: | |
| 370 | 370 | - `sudo pacman -S gst-plugins-base gst-plugins-good gst-plugins-bad gst-plugins-ugly gst-libav gst-plugin-pipewire pipewire pipewire-pulse` | |
| 371 | - | - Hardware accel: `gstreamer-vaapi libva-mesa-driver` (AMD), `libva-vdpau-driver` (NVIDIA), `intel-media-driver` (Intel) | |
| 371 | + | - Hardware accel: `gstreamer-vaapi mesa` (AMD), `libva-nvidia-driver` (NVIDIA), `libva-intel-driver intel-media-driver` (Intel) | |
| 372 | 372 | - Debian/Ubuntu: | |
| 373 | 373 | - `sudo apt install gstreamer1.0-tools gstreamer1.0-plugins-base gstreamer1.0-plugins-good gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly gstreamer1.0-libav gstreamer1.0-pipewire` | |
| 374 | 374 | - Hardware accel: `gstreamer1.0-vaapi va-driver-all` (covers most GPUs) | |
| @@ -368,7 +368,7 @@ WebKitGTK uses GStreamer for media playback. Dumber includes automatic hardware | |||||
| 368 | **Required packages:** | 368 | **Required packages:** | ||
| 369 | - Arch Linux: | 369 | - Arch Linux: | ||
| 370 | - `sudo pacman -S gst-plugins-base gst-plugins-good gst-plugins-bad gst-plugins-ugly gst-libav gst-plugin-pipewire pipewire pipewire-pulse` | 370 | - `sudo pacman -S gst-plugins-base gst-plugins-good gst-plugins-bad gst-plugins-ugly gst-libav gst-plugin-pipewire pipewire pipewire-pulse` | ||
| 371 | - | - Hardware accel: `gstreamer-vaapi libva-mesa-driver` (AMD), `libva-vdpau-driver` (NVIDIA), `intel-media-driver` (Intel) | 371 | + | - Hardware accel: `gstreamer-vaapi mesa` (AMD), `libva-nvidia-driver` (NVIDIA), `libva-intel-driver intel-media-driver` (Intel) |
| 372 | - Debian/Ubuntu: | 372 | - Debian/Ubuntu: | ||
| 373 | - `sudo apt install gstreamer1.0-tools gstreamer1.0-plugins-base gstreamer1.0-plugins-good gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly gstreamer1.0-libav gstreamer1.0-pipewire` | 373 | - `sudo apt install gstreamer1.0-tools gstreamer1.0-plugins-base gstreamer1.0-plugins-good gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly gstreamer1.0-libav gstreamer1.0-pipewire` | ||
| 374 | - Hardware accel: `gstreamer1.0-vaapi va-driver-all` (covers most GPUs) | 374 | - Hardware accel: `gstreamer1.0-vaapi va-driver-all` (covers most GPUs) | ||
A docker-compose.yml +38 -0
| @@ -0,0 +1,38 @@ | |||
| 1 | + | # Docker Compose configuration for goreleaser builds | |
| 2 | + | ||
| 3 | + | services: | |
| 4 | + | # Goreleaser service for building releases | |
| 5 | + | goreleaser: | |
| 6 | + | build: | |
| 7 | + | context: . | |
| 8 | + | dockerfile: Dockerfile.release | |
| 9 | + | volumes: | |
| 10 | + | - .:/workspace | |
| 11 | + | environment: | |
| 12 | + | - GITHUB_TOKEN | |
| 13 | + | working_dir: /workspace | |
| 14 | + | command: build --snapshot --clean | |
| 15 | + | network_mode: host | |
| 16 | + | ||
| 17 | + | # Service for full release (with git tag) | |
| 18 | + | release: | |
| 19 | + | build: | |
| 20 | + | context: . | |
| 21 | + | dockerfile: Dockerfile.release | |
| 22 | + | volumes: | |
| 23 | + | - .:/workspace | |
| 24 | + | environment: | |
| 25 | + | - GITHUB_TOKEN | |
| 26 | + | working_dir: /workspace | |
| 27 | + | command: release --clean | |
| 28 | + | network_mode: host | |
| 29 | + | ||
| 30 | + | # Service for validation only | |
| 31 | + | check: | |
| 32 | + | build: | |
| 33 | + | context: . | |
| 34 | + | dockerfile: Dockerfile.release | |
| 35 | + | volumes: | |
| 36 | + | - .:/workspace | |
| 37 | + | working_dir: /workspace | |
| 38 | + | command: check | |
| @@ -0,0 +1,38 @@ | |||||
| 1 | + | # Docker Compose configuration for goreleaser builds | |||
| 2 | + | ||||
| 3 | + | services: | |||
| 4 | + | # Goreleaser service for building releases | |||
| 5 | + | goreleaser: | |||
| 6 | + | build: | |||
| 7 | + | context: . | |||
| 8 | + | dockerfile: Dockerfile.release | |||
| 9 | + | volumes: | |||
| 10 | + | - .:/workspace | |||
| 11 | + | environment: | |||
| 12 | + | - GITHUB_TOKEN | |||
| 13 | + | working_dir: /workspace | |||
| 14 | + | command: build --snapshot --clean | |||
| 15 | + | network_mode: host | |||
| 16 | + | ||||
| 17 | + | # Service for full release (with git tag) | |||
| 18 | + | release: | |||
| 19 | + | build: | |||
| 20 | + | context: . | |||
| 21 | + | dockerfile: Dockerfile.release | |||
| 22 | + | volumes: | |||
| 23 | + | - .:/workspace | |||
| 24 | + | environment: | |||
| 25 | + | - GITHUB_TOKEN | |||
| 26 | + | working_dir: /workspace | |||
| 27 | + | command: release --clean | |||
| 28 | + | network_mode: host | |||
| 29 | + | ||||
| 30 | + | # Service for validation only | |||
| 31 | + | check: | |||
| 32 | + | build: | |||
| 33 | + | context: . | |||
| 34 | + | dockerfile: Dockerfile.release | |||
| 35 | + | volumes: | |||
| 36 | + | - .:/workspace | |||
| 37 | + | working_dir: /workspace | |||
| 38 | + | command: check | |||
M internal/logging/capture.go +6 -5
| @@ -6,8 +6,9 @@ import ( | |||
| 6 | 6 | "io" | |
| 7 | 7 | "os" | |
| 8 | 8 | "strings" | |
| 9 | - | "syscall" | |
| 10 | 9 | "time" | |
| 10 | + | ||
| 11 | + | "golang.org/x/sys/unix" | |
| 11 | 12 | ) | |
| 12 | 13 | ||
| 13 | 14 | type OutputCapture struct { | |
| @@ -61,8 +62,8 @@ func (c *OutputCapture) Start() error { | |||
| 61 | 62 | os.Stderr = stderrW | |
| 62 | 63 | ||
| 63 | 64 | // Also redirect file descriptors at syscall level for C code | |
| 64 | - | syscall.Dup2(int(stdoutW.Fd()), 1) | |
| 65 | - | syscall.Dup2(int(stderrW.Fd()), 2) | |
| 65 | + | unix.Dup3(int(stdoutW.Fd()), 1, 0) | |
| 66 | + | unix.Dup3(int(stderrW.Fd()), 2, 0) | |
| 66 | 67 | ||
| 67 | 68 | // Start goroutines to read and log | |
| 68 | 69 | go c.pipeToLogger(stdoutR, "STDOUT") | |
| @@ -84,8 +85,8 @@ func (c *OutputCapture) Stop() { | |||
| 84 | 85 | os.Stderr = c.originalStderr | |
| 85 | 86 | ||
| 86 | 87 | // Restore file descriptors | |
| 87 | - | syscall.Dup2(int(c.originalStdout.Fd()), 1) | |
| 88 | - | syscall.Dup2(int(c.originalStderr.Fd()), 2) | |
| 88 | + | unix.Dup3(int(c.originalStdout.Fd()), 1, 0) | |
| 89 | + | unix.Dup3(int(c.originalStderr.Fd()), 2, 0) | |
| 89 | 90 | ||
| 90 | 91 | // Close pipes | |
| 91 | 92 | if c.stdoutWrite != nil { | |
| @@ -6,8 +6,9 @@ import ( | |||||
| 6 | "io" | 6 | "io" | ||
| 7 | "os" | 7 | "os" | ||
| 8 | "strings" | 8 | "strings" | ||
| 9 | - | "syscall" | |||
| 10 | "time" | 9 | "time" | ||
| 10 | + | ||||
| 11 | + | "golang.org/x/sys/unix" | |||
| 11 | ) | 12 | ) | ||
| 12 | 13 | ||||
| 13 | type OutputCapture struct { | 14 | type OutputCapture struct { | ||
| @@ -61,8 +62,8 @@ func (c *OutputCapture) Start() error { | |||||
| 61 | os.Stderr = stderrW | 62 | os.Stderr = stderrW | ||
| 62 | 63 | ||||
| 63 | // Also redirect file descriptors at syscall level for C code | 64 | // Also redirect file descriptors at syscall level for C code | ||
| 64 | - | syscall.Dup2(int(stdoutW.Fd()), 1) | 65 | + | unix.Dup3(int(stdoutW.Fd()), 1, 0) |
| 65 | - | syscall.Dup2(int(stderrW.Fd()), 2) | 66 | + | unix.Dup3(int(stderrW.Fd()), 2, 0) |
| 66 | 67 | ||||
| 67 | // Start goroutines to read and log | 68 | // Start goroutines to read and log | ||
| 68 | go c.pipeToLogger(stdoutR, "STDOUT") | 69 | go c.pipeToLogger(stdoutR, "STDOUT") | ||
| @@ -84,8 +85,8 @@ func (c *OutputCapture) Stop() { | |||||
| 84 | os.Stderr = c.originalStderr | 85 | os.Stderr = c.originalStderr | ||
| 85 | 86 | ||||
| 86 | // Restore file descriptors | 87 | // Restore file descriptors | ||
| 87 | - | syscall.Dup2(int(c.originalStdout.Fd()), 1) | 88 | + | unix.Dup3(int(c.originalStdout.Fd()), 1, 0) |
| 88 | - | syscall.Dup2(int(c.originalStderr.Fd()), 2) | 89 | + | unix.Dup3(int(c.originalStderr.Fd()), 2, 0) |
| 89 | 90 | ||||
| 90 | // Close pipes | 91 | // Close pipes | ||
| 91 | if c.stdoutWrite != nil { | 92 | if c.stdoutWrite != nil { | ||