Add webkitgtk6 option #4570
Add webkitgtk6 option #4570
Conversation
`webkit_web_view_new_with_user_content_manager` has been removed in webkit-6 so this now has to be done manually.
Gtk4 removed several menu widgets and reworked menu and accelerator interactions.
This just removes some already disabled code and clarifies some comments.
This doesn't fully implement functionality and may fail. See: https://discourse.gnome.org/t/file-drag-and-drop-in-gtkmm4/10548/5
Gtk4 requires an app id. The existing 'ProgramName' app option seems like a good fit for this.
The 'on demand' policy was removed but 'never' should still map to case 2.
WalkthroughAdds build-tagged support for WebKit 6 / GTK4 alongside existing WebKit2/GTK3 paths. Introduces new GTK4/WebKit6 implementations (window, menu, screen, clipboard, main-loop), many CGO/C helpers and headers, and adjusts pkg-config/cgo build constraints across frontend and assetserver webview files. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Go App
participant CGO as CGO bridge (initGtk / runMainLoop / setMainLoop)
participant GTK as GTK runtime (gtk3 or gtk4)
participant Window as Window (C/Go bridge)
participant WebView as WebView (WebKit2 or WebKit6)
participant Backend as Backend (Go)
App->>CGO: initGtk()
CGO-->>GTK: initialize GTK (gtk3 or gtk4) based on WEBKIT_6
App->>Window: NewWindow / Run
Window->>WebView: SetupWebview()
WebView->>Backend: script-message -> processMessage (via CGO)
Backend-->>WebView: respond / ExecuteJS
Note right of Window: Menu, Clipboard, Screen use conditional WEBKIT_6 paths
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)v2/internal/frontend/desktop/linux/window_webkit6.c (2)
v2/internal/frontend/desktop/linux/window_webkit6.h (1)
🪛 Clang (14.0.6)v2/internal/frontend/desktop/linux/window_webkit6.c[error] 4-4: 'jsc/jsc.h' file not found (clang-diagnostic-error) v2/internal/frontend/desktop/linux/window_webkit6.h[error] 7-7: 'jsc/jsc.h' file not found (clang-diagnostic-error) 🔇 Additional comments (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Also need changes for doctor command
https://github.com/Sammy-T/wails/tree/feature/webkitgtk6/v2/internal/system/packagemanager
|
@COD3HUNT3R this is intended to work similarly to when using |
Yes, you are right but this would be helpful to find required packages for gtk4, webkit2-6.0 |
|
Had to revert updating from master (ca1eed6) because it broke local development on linux for me. |
|
Running this on X11 Linux Mint, this appears to be working nicely. I still don't know how/if I can get the todos (window positioning, etc.) working with webkit-6 though. @leaanthony Are you able to test this and see how it runs for you? And do you know how to address the commented todos? |
Sammy-T
force-pushed
the
feature/webkitgtk6
branch
from
b0e94be to
66060e8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
v2/internal/frontend/desktop/linux/window_webkit6.c (1)
45-81: LGTM! Message handling and Wayland detection are correct.The WebKit message callback properly converts JSC values to strings and frees memory. Wayland detection correctly checks
XDG_SESSION_TYPEand caches the result. Minor formatting: line 67 has inconsistent indentation (missing leading spaces).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v2/internal/frontend/desktop/linux/window_webkit6.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
v2/internal/frontend/desktop/linux/window_webkit6.c (3)
v2/internal/frontend/runtime/desktop/events.js (1)
data(105-105)v2/internal/frontend/runtime/desktop/draganddrop.js (1)
files(136-136)v2/internal/frontend/desktop/linux/dialog.go (1)
GTK_FILE_CHOOSER_ACTION_SAVE(19-19)
🔇 Additional comments (25)
v2/internal/frontend/desktop/linux/window_webkit6.c (25)
1-12: LGTM! Build tags and includes are correct.The build tags properly restrict this implementation to Linux with the
webkit_6tag, and the includes cover GTK4, WebKit6, and JavaScriptCore APIs.
14-43: LGTM! Static state and helper functions are appropriate.The static variables correctly track mouse drag state, and the cast helpers provide clean type conversions for GTK objects.
ExecuteOnMainThreadproperly usesg_idle_addto marshal work to the GTK main thread.
83-121: LGTM! Monitor geometry helpers are well-implemented.The functions correctly query GTK4's native surface and display APIs to retrieve monitor information, with appropriate null checks and error handling.
125-128: LGTM! Signal setup is correct.The function properly connects the WebKit content manager's script message signal and returns the handler ID for later management.
130-147: SetWindowIcon remains a non-functional stub.As noted in previous reviews, this function is entirely commented out and does not set the application icon. This is a known limitation of the current GTK4/webkit_6 implementation.
149-161: SetWindowTransparency remains a non-functional stub.As flagged in previous reviews, this function is commented out. Translucent windows (
appoptions.Linux.WindowIsTranslucent) will not work correctly without this implementation.
163-212: LGTM! Background color handling is well-implemented.The function correctly sets both webview and window background colors. The locale manipulation ensures consistent float formatting, and the CSS provider singleton pattern is appropriate. The TODO on line 205 acknowledges the deprecation of
gtk_css_provider_load_from_datasince GTK 4.12, which is acceptable for backward compatibility.
214-230: LGTM! Title setting is correctly implemented.The function properly marshals title updates to the main thread with correct memory management—both the title string and args struct are freed in the callback.
232-257: SetPosition remains a non-functional stub.As noted in previous reviews,
gtk_window_movewas removed in GTK4, so programmatic window positioning is not supported in webkit_6.
259-299: SetMinMaxSize remains a non-functional stub.As flagged in previous reviews,
gtk_window_set_geometry_hintswas removed in GTK4. This meansappoptions.MinWidth,MinHeight,MaxWidth, andMaxHeightare not enforced in webkit_6 builds.
301-312: LGTM! Context menu disabling is correct.The callback correctly returns
TRUEto suppress the default WebKit context menu while allowing event propagation.
314-369: LGTM! Button event handling is correctly implemented.The gesture controllers properly capture mouse button, position, and timestamp for window drag/resize operations. The implementation correctly uses GTK4's gesture API and only tracks left-button clicks.
389-413: Center remains a non-functional stub.As noted in previous reviews, window centering is not functional because
gtk_window_movewas removed in GTK4. The Go code may still call this function, but it has no effect.
415-480: LGTM! Window state transitions are correctly implemented.The functions properly use GTK4 APIs for show/hide, maximize/unmaximize, minimize/present, and fullscreen operations. Note that
FullscreencallsSetMinMaxSize(line 468), which is currently a no-op stub, but this doesn't impact fullscreen functionality since the window should fill the screen regardless.
482-504: LGTM! WebView load and window lifecycle callbacks are correct.The load-changed handler properly signals "DomReady" on completion. The close button handler correctly sends "Q" and returns
TRUEto prevent default close behavior.
506-585: Drag-and-drop remains incomplete.As noted in previous reviews, the drag-and-drop implementation is non-functional. The old implementation is commented out (lines 508-570), and the current stub (lines 574-585) only prints file paths without processing them.
587-641: LGTM! WebView setup is well-implemented.The function correctly initializes the WebKitWebView, registers the
wails://URI scheme, configures the user agent, and sets up hardware acceleration policies. The hide-on-close logic appropriately chooses between hiding or processing close events.
643-658: LGTM! Developer tools and URL loading are correctly implemented.The functions properly enable WebKit developer extras and load URIs using the appropriate WebKit6 APIs.
660-685: Incorrect cast in gdk_toplevel_begin_move.As flagged in previous reviews, line 672 passes
options->mainwindow(aGtkWindow*) where aGdkToplevel*is expected. The correct approach is to obtain theGdkSurfaceviagtk_native_get_surface(GTK_NATIVE(options->mainwindow))and cast it toGDK_TOPLEVEL(surface).
687-712: Incorrect cast in gdk_toplevel_begin_resize.As flagged in previous reviews, line 699 has the same casting issue as line 672. The
options->mainwindowmust be converted toGdkToplevel*by obtaining theGdkSurfacefirst.
722-788: LGTM! Message dialog implementation is correct.The function properly handles different message types and buttons, with correct memory management. The TODO on line 766 appropriately acknowledges that
gtk_message_dialog_newis deprecated since GTK 4.10, which is acceptable for backward compatibility.
790-843: LGTM! File dialog result handling is correct.The function properly iterates through selected files using GTK4's
GListModelAPI, with correct memory management. The 1024-file limit (line 823) is an arbitrary but reasonable constraint.
845-932: LGTM! File chooser dialog is well-implemented.The function correctly creates and configures a file chooser dialog with proper memory management. Multiple TODOs (lines 854, 865, 896) appropriately acknowledge GTK 4.10 deprecations while maintaining backward compatibility.
934-978: LGTM! Inspector and application lifecycle functions are correctly implemented.The functions properly handle WebKit inspector display, F12 hotkey registration, and GTK application creation/execution. The reference counting and signal connections are all correct.
714-720: Remove incorrectfree(js)suggestion.The JSCallback struct is stack-allocated in Go’s ExecJS and must not be freed in ExecuteJS; freeing only
js->scriptis correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
v2/internal/frontend/desktop/linux/window_webkit6.go (2)
234-246: Document the GetPosition limitation for GTK4The TODO comment notes that
gtk_window_get_positionwas removed in GTK4, and the function returns(0, 0)as a placeholder. Consider documenting this limitation in the function's godoc so callers are aware of the behavior difference from the WebKit2 implementation.Add a comment above the function:
+// GetPosition returns the window position. Note: GTK4 removed gtk_window_get_position, +// so this always returns (0, 0). Callers should not rely on this for WebKit6 builds. func (w *Window) GetPosition() (int, int) { //// TODO: gtk_window_get_position was removed in gtk4
67-68: Rename validation function to match WebKit6 usage
The call in window_webkit6.go still invokesvalidateWebKit2Version—either rename this function to something generic (e.g.,validateWebKitVersion) or add avalidateWebKit6Versionvariant and update its callers.v2/internal/frontend/desktop/linux/window_webkit6.h (1)
88-93: Document stubbed window functions in headerAdd a note in
v2/internal/frontend/desktop/linux/window_webkit6.hthat the following APIs are currently unimplemented no-ops under WebKit6/GTK4:
SetWindowIconSetWindowTransparencySetPositionSetMinMaxSizeOptionally, emit a runtime warning (e.g. via
g_warning) in each C stub to notify users when these functions are invoked.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v2/internal/frontend/desktop/linux/gtk_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/menu_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
v2/internal/frontend/desktop/linux/gtk_webkit6.go (1)
v2/pkg/menu/callback.go (1)
CallbackData(3-6)
v2/internal/frontend/desktop/linux/window_webkit6.go (2)
v2/internal/frontend/desktop/linux/window.go (2)
NewWindow(62-149)Window(40-53)v2/internal/frontend/desktop/linux/window_webkit6.c (33)
SetupInvokeSignal(125-128)SetupWebview(590-641)ConnectButtons(359-369)DevtoolsEnabled(643-653)InstallF12Hotkey(951-961)DisableContextMenu(308-312)SetBackgroundColour(165-212)SetTitle(224-230)SetWindowIcon(131-147)SetWindowTransparency(149-161)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)Fullscreen(457-473)ExecuteOnMainThread(25-28)Center(391-413)SetPosition(245-257)SetMinMaxSize(260-299)Show(415-420)Hide(422-427)Maximise(429-434)Minimise(443-448)IsFullscreen(371-375)IsMaximised(377-381)IsMinimised(383-387)LoadIndex(655-658)ExecuteJS(714-720)StartDrag(679-685)StartResize(705-712)AllocFileFilterArray(792-795)Opendialog(845-932)MessageDialog(740-788)ShowInspector(941-944)
v2/internal/frontend/desktop/linux/window_webkit6.h (1)
v2/internal/frontend/desktop/linux/window_webkit6.c (39)
ExecuteOnMainThread(25-28)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)SetupInvokeSignal(125-128)SetWindowIcon(131-147)SetWindowTransparency(149-161)SetBackgroundColour(165-212)SetTitle(224-230)SetPosition(245-257)SetMinMaxSize(260-299)DisableContextMenu(308-312)ConnectButtons(359-369)IsFullscreen(371-375)IsMaximised(377-381)IsMinimised(383-387)Center(391-413)Show(415-420)Hide(422-427)Maximise(429-434)UnMaximise(436-441)Minimise(443-448)UnMinimise(450-455)Fullscreen(457-473)UnFullscreen(475-480)SetupWebview(590-641)LoadIndex(655-658)DevtoolsEnabled(643-653)ExecuteJS(714-720)StartDrag(679-685)StartResize(705-712)MessageDialog(740-788)AllocFileFilterArray(792-795)Opendialog(845-932)sendShowInspectorMessage(946-948)ShowInspector(941-944)InstallF12Hotkey(951-961)createApp(969-973)runApp(975-978)
🔇 Additional comments (10)
v2/internal/frontend/desktop/linux/menu_webkit6.go (2)
73-77: LGTM: Proper concurrency-safe map usageThe conversion to
sync.MapforgActionIdToMenuItemcorrectly addresses the concurrent access concerns raised in previous reviews, as this map is written here and read from CGO callbacks ingtk_webkit6.go.
87-97: Potential memory leak: Previous menu resources not freedWhen
SetApplicationMenuis called multiple times (e.g., during menu updates), the old GTK/GMenu objects cached ingtkMenuCacheand the existingwindow.menubarare not freed before resetting the maps. This leaks native GTK resources.Before resetting the maps, unref all cached GMenu objects and destroy the existing menubar:
func (w *Window) SetApplicationMenu(inmenu *menu.Menu) { if inmenu == nil { return } + // Clean up previous menu resources + for _, gmenu := range gtkMenuCache { + if gmenu != nil { + C.g_object_unref(C.gpointer(gmenu)) + } + } + if w.menubar != nil { + C.gtk_widget_unparent(w.menubar) + w.menubar = nil + } + menuItemToId = make(map[*menu.MenuItem]int) menuIdToItem = make(map[int]*menu.MenuItem) gtkMenuCache = make(map[*menu.MenuItem]*C.GMenu) processMenu(w, inmenu) }v2/internal/frontend/desktop/linux/window_webkit6.go (2)
67-73: Good validation: GTK4 requires ProgramNameThe fatal error when
Linux.ProgramNameis missing is appropriate because GTK4 applications require a valid application ID. This makes the requirement explicit to developers.
405-414: LGTM: Proper nil guard prevents double-unrefThe nil check and subsequent assignment to nil correctly prevent double-unref issues if
Quit()is called multiple times.v2/internal/frontend/desktop/linux/window_webkit6.h (6)
127-127: Previous issue resolved: function signature now correct.The declaration for
sendShowInspectorMessagenow correctly matches the implementation signature with parameters(GAction *action, GVariant *param).
7-12: LGTM: Includes are appropriate for WebKit6/GTK4.The header includes cover JavaScript core, GTK4, WebKit, and standard C libraries as needed.
14-76: LGTM: Struct typedefs are well-defined.All typedef structs are correctly defined with appropriate field types for GTK4/WebKit6 integration.
82-82: Correct decision to comment out GtkContainer.
GtkContainerwas removed in GTK4, so commenting out this declaration is appropriate for the webkit_6 build path.
131-132: LGTM: GTK4 application lifecycle functions.
createAppandrunAppcorrectly expose the GTK4 GtkApplication lifecycle management, which replaces the deprecatedgtk_mainapproach from GTK3.
128-129: LGTM: Inspector function declarations match implementations.The inspector functions (
ShowInspector,InstallF12Hotkey) are correctly declared and match their implementations inwindow_webkit6.c.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
v2/internal/frontend/desktop/linux/window_webkit6.go (1)
133-135: Unused variable:buttonPressedNameallocated but never used.Line 133 allocates a C string
"button-press-event"that is deferred-freed on line 134, but the variable is never passed to any function. This appears to be leftover code.Apply this diff to remove the unused allocation:
result.webview = unsafe.Pointer(webview) - buttonPressedName := C.CString("button-press-event") - defer C.free(unsafe.Pointer(buttonPressedName)) C.ConnectButtons(unsafe.Pointer(webview))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v2/internal/frontend/desktop/linux/window_webkit6.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
v2/internal/frontend/desktop/linux/window_webkit6.go (2)
v2/internal/frontend/desktop/linux/window.go (2)
NewWindow(62-149)Window(40-53)v2/internal/frontend/desktop/linux/window_webkit6.c (31)
SetupInvokeSignal(125-128)SetupWebview(590-641)ConnectButtons(359-369)DevtoolsEnabled(643-653)InstallF12Hotkey(951-961)DisableContextMenu(308-312)SetBackgroundColour(165-212)SetTitle(224-230)SetWindowIcon(131-147)SetWindowTransparency(149-161)Fullscreen(457-473)ExecuteOnMainThread(25-28)UnFullscreen(475-480)Center(391-413)SetPosition(245-257)SetMinMaxSize(260-299)Show(415-420)Hide(422-427)Maximise(429-434)Minimise(443-448)IsFullscreen(371-375)IsMaximised(377-381)IsMinimised(383-387)LoadIndex(655-658)ExecuteJS(714-720)StartDrag(679-685)StartResize(705-712)AllocFileFilterArray(792-795)Opendialog(845-932)MessageDialog(740-788)ShowInspector(941-944)
🔇 Additional comments (7)
v2/internal/frontend/desktop/linux/window_webkit6.go (7)
383-387: Memory management fixed correctly.The previous memory leak issue has been resolved. The C string is now allocated once, properly passed to
SetTitle, and freed via defer.
405-414: Double-unref protection implemented correctly.The previous issue has been addressed. The method now guards against double-unref by checking for nil and resetting
mainLoopafter cleanup.
459-465: Verify C function frees allocated strings.Lines 460 and 464 allocate C strings for
defaultFilenameanddefaultDirectorywithout immediatedefer C.free. These are passed to the C functionOpendialog.Looking at the C implementation (lines 844-931 in window_webkit6.c), lines 853 and 868 show
free(options->defaultDirectory)andfree(options->defaultFilename), confirming the C side handles cleanup.The memory ownership transfer is correct - C side frees these strings.
470-488: Memory management correct - C function frees strings.The C strings for title and message are properly freed by the C side (lines 784-785 in window_webkit6.c).
389-395: ExecJS allocation is freed; no memory leak here. C.ExecuteJS unconditionally callsfree(js->script)after evaluating the script, so the C.CString allocation inExecJSis properly released.Likely an incorrect or invalid review comment.
60-65: No action needed foractivateWgsynchronization. The package-levelactivateWgis called exactly once at startup in Linux (frontend.go), so there’s no concurrent reuse.
234-246: Linux WebKit6GetPositionalways returns (0, 0). All calls toWindowGetPositionon Linux with thewebkit_6build tag yield zero and there’s no downstream handling for this stub. Please document this limitation in the API or provide an alternative implementation.
| func (w *Window) asGTKBox() *C.GtkBox { | ||
| return C.GTKBOX(w.gtkWindow) | ||
| } |
There was a problem hiding this comment.
Incorrect type cast: asGTKBox casts gtkWindow instead of a box widget.
Line 183 casts w.gtkWindow to *C.GtkBox, but gtkWindow is a GtkWindow, not a box. This will cause type errors if the method is ever called. Likely should cast w.vbox or w.webviewBox.
Apply this diff to fix the cast:
func (w *Window) asGTKBox() *C.GtkBox {
- return C.GTKBOX(w.gtkWindow)
+ return C.GTKBOX(unsafe.Pointer(w.vbox))
}🤖 Prompt for AI Agents
In v2/internal/frontend/desktop/linux/window_webkit6.go around lines 182 to 184,
the asGTKBox method incorrectly casts w.gtkWindow to *C.GtkBox; change the cast
to the actual box field (e.g., w.vbox) so it returns C.GTKBOX(w.vbox) (or
w.webviewBox if that is the intended container), ensuring the returned GtkBox is
the correct widget rather than the GtkWindow.
|
|
||
| C.gtk_window_present(w.asGTKWindow()) | ||
|
|
||
| w.Center() |
There was a problem hiding this comment.
Center() call may be ineffective.
The C implementation of Center (lines 390-412 in window_webkit6.c from relevant snippets) is entirely commented out. This call does nothing and the window won't be centered.
Consider removing this call or implementing centering via GTK4 APIs:
gtk_window_present(w.asGTKWindow())
- w.Center()
switch w.appoptions.WindowStartState {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| w.Center() | |
| gtk_window_present(w.asGTKWindow()) | |
| switch w.appoptions.WindowStartState { |
🤖 Prompt for AI Agents
In v2/internal/frontend/desktop/linux/window_webkit6.go around line 351, the
call to w.Center() is ineffective because the C implementation is commented out;
either remove this no-op call or replace it with a real GTK4-based centering
implementation: obtain the monitor workarea from the GDK display/monitor APIs,
compute centered x/y from the monitor workarea and the window size, and position
the window using the appropriate GTK4/GDK window movement API (or set window
size/constraints to achieve the same), ensuring any C bindings are updated to
call the GTK4 functions instead of the commented-out C Center implementation.
| // showModalDialogAndExit shows a modal dialog and exits the app. | ||
| func showModalDialogAndExit(title, message string) { | ||
| go func() { | ||
| data := C.MessageDialogOptions{ | ||
| title: C.CString(title), | ||
| message: C.CString(message), | ||
| messageType: C.int(1), | ||
| } | ||
|
|
||
| C.MessageDialog(unsafe.Pointer(&data)) | ||
| }() | ||
|
|
||
| <-messageDialogResult | ||
| log.Fatal(message) | ||
| } |
There was a problem hiding this comment.
Potential threading issue: C.MessageDialog not called on main thread.
Line 511 calls C.MessageDialog directly within a goroutine without using invokeOnMainThread. GTK functions must be called on the main thread. This could cause crashes or undefined behavior.
Apply this diff to fix:
- C.MessageDialog(unsafe.Pointer(&data))
+ invokeOnMainThread(func() { C.MessageDialog(unsafe.Pointer(&data)) })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // showModalDialogAndExit shows a modal dialog and exits the app. | |
| func showModalDialogAndExit(title, message string) { | |
| go func() { | |
| data := C.MessageDialogOptions{ | |
| title: C.CString(title), | |
| message: C.CString(message), | |
| messageType: C.int(1), | |
| } | |
| C.MessageDialog(unsafe.Pointer(&data)) | |
| }() | |
| <-messageDialogResult | |
| log.Fatal(message) | |
| } | |
| // showModalDialogAndExit shows a modal dialog and exits the app. | |
| func showModalDialogAndExit(title, message string) { | |
| go func() { | |
| data := C.MessageDialogOptions{ | |
| title: C.CString(title), | |
| message: C.CString(message), | |
| messageType: C.int(1), | |
| } | |
| invokeOnMainThread(func() { C.MessageDialog(unsafe.Pointer(&data)) }) | |
| }() | |
| <-messageDialogResult | |
| log.Fatal(message) | |
| } |
🤖 Prompt for AI Agents
In v2/internal/frontend/desktop/linux/window_webkit6.go around lines 502 to 516,
the code spawns a goroutine that calls C.MessageDialog which must run on the
main GTK thread; change the goroutine to call C.MessageDialog via the existing
invokeOnMainThread (or equivalent main-thread dispatcher) so the C.MessageDialog
invocation happens on the main thread, and ensure any C.CString allocations are
still created before dispatch and freed afterwards on the main thread as part of
that dispatched call.
Applying the original suggestion didn't account for it being freed from C but at least there's a comment to help indicate it now.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2/internal/frontend/desktop/linux/menu_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
v2/internal/frontend/desktop/linux/menu_webkit6.go (3)
v2/pkg/runtime/menu.go (2)
MenuSetApplicationMenu(9-12)MenuUpdateApplicationMenu(14-17)v2/internal/frontend/desktop/linux/window_webkit6.go (1)
Window(38-51)v2/pkg/menu/menuitem.go (1)
SubMenu(319-329)
v2/internal/frontend/desktop/linux/window_webkit6.go (2)
v2/internal/frontend/desktop/linux/window.go (2)
NewWindow(62-149)Window(40-53)v2/internal/frontend/desktop/linux/window_webkit6.c (35)
SetupInvokeSignal(125-128)SetupWebview(590-641)ConnectButtons(359-369)DevtoolsEnabled(643-653)InstallF12Hotkey(951-961)DisableContextMenu(308-312)SetBackgroundColour(165-212)SetTitle(224-230)SetWindowIcon(131-147)SetWindowTransparency(149-161)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)Fullscreen(457-473)ExecuteOnMainThread(25-28)UnFullscreen(475-480)Center(391-413)SetMinMaxSize(260-299)Show(415-420)Hide(422-427)Maximise(429-434)UnMaximise(436-441)Minimise(443-448)UnMinimise(450-455)IsFullscreen(371-375)IsMaximised(377-381)IsMinimised(383-387)LoadIndex(655-658)ExecuteJS(714-720)StartDrag(679-685)StartResize(705-712)AllocFileFilterArray(792-795)Opendialog(845-932)MessageDialog(740-788)ShowInspector(941-944)
| func (w *Window) SetApplicationMenu(inmenu *menu.Menu) { | ||
| if inmenu == nil { | ||
| return | ||
| } | ||
|
|
||
| // Clean up previous menu | ||
| for _, gmenu := range gtkMenuCache { | ||
| if gmenu != nil { | ||
| C.g_object_unref(C.gpointer(gmenu)) | ||
| } | ||
| } | ||
|
|
||
| if w.menubar != nil { | ||
| C.gtk_widget_unparent(w.menubar) | ||
| w.menubar = nil | ||
| } | ||
|
|
||
| menuItemToId = make(map[*menu.MenuItem]int) | ||
| menuIdToItem = make(map[int]*menu.MenuItem) | ||
| gtkMenuCache = make(map[*menu.MenuItem]*C.GMenu) | ||
|
|
||
| processMenu(w, inmenu) | ||
| } |
There was a problem hiding this comment.
Persist the application menu before rebuilding
Line 87 returns immediately when inmenu is nil, and nowhere else do we assign to w.applicationMenu. As a result, MenuUpdateApplicationMenu() always sees a nil menu and becomes a no-op after the first call. Please store the menu pointer up front so refreshes keep working.
func (w *Window) SetApplicationMenu(inmenu *menu.Menu) {
+ w.applicationMenu = inmenu
+
if inmenu == nil {
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (w *Window) SetApplicationMenu(inmenu *menu.Menu) { | |
| if inmenu == nil { | |
| return | |
| } | |
| // Clean up previous menu | |
| for _, gmenu := range gtkMenuCache { | |
| if gmenu != nil { | |
| C.g_object_unref(C.gpointer(gmenu)) | |
| } | |
| } | |
| if w.menubar != nil { | |
| C.gtk_widget_unparent(w.menubar) | |
| w.menubar = nil | |
| } | |
| menuItemToId = make(map[*menu.MenuItem]int) | |
| menuIdToItem = make(map[int]*menu.MenuItem) | |
| gtkMenuCache = make(map[*menu.MenuItem]*C.GMenu) | |
| processMenu(w, inmenu) | |
| } | |
| func (w *Window) SetApplicationMenu(inmenu *menu.Menu) { | |
| // Persist for future updates | |
| w.applicationMenu = inmenu | |
| if inmenu == nil { | |
| return | |
| } | |
| // Clean up previous menu | |
| for _, gmenu := range gtkMenuCache { | |
| if gmenu != nil { | |
| C.g_object_unref(C.gpointer(gmenu)) | |
| } | |
| } | |
| if w.menubar != nil { | |
| C.gtk_widget_unparent(w.menubar) | |
| w.menubar = nil | |
| } | |
| menuItemToId = make(map[*menu.MenuItem]int) | |
| menuIdToItem = make(map[int]*menu.MenuItem) | |
| gtkMenuCache = make(map[*menu.MenuItem]*C.GMenu) | |
| processMenu(w, inmenu) | |
| } |
🤖 Prompt for AI Agents
v2/internal/frontend/desktop/linux/menu_webkit6.go around lines 87-109: the
function currently returns immediately when inmenu is nil and never assigns
w.applicationMenu, so future refreshes see nil; fix by persisting the incoming
menu pointer at the top (set w.applicationMenu = inmenu), and if inmenu is nil
clear any existing native menu state/caches and return; otherwise continue with
the existing cleanup and rebuild logic so MenuUpdateApplicationMenu() can find
the stored menu on subsequent calls.
|
@Sammy-T - Thanks for your continued effort on this 🙏 Is it still a WIP? Wondering if we should convert this to draft? |
|
@leaanthony Thanks. It's been a lot of work and I wasn't sure I'd even get this far. Sorry if I've been spamming you. I wasn't expecting to get hit with all those suggestions. Some of them were relevant. I can switch it back to draft. Currently, this PR is working with the exception of missing some of the window control that was removed in gtk4 (I've marked them in the code as todo comments). I was hoping you could test it out and also take a look at the todos to see if you know of any workarounds/updates? |
This sets the transparency for the entire widget including the menu and the webview's contents.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (5)
v2/internal/frontend/desktop/linux/window_webkit6.c (2)
387-411: Center/SetPosition/SetMinMaxSize are no-ops; document or feature-flag.Calls from Go currently give the impression features exist.
- Add explicit comments in Go calling sites noting these are no-ops on GTK4, or guard with build-tagged capabilities.
Also applies to: 235-249, 251-291
636-641: Incorrect argument type for gdk_toplevel_begin_move/resize.Pass a
GdkToplevel*from the window’s surface, notGtkWindow*.- gdk_toplevel_begin_move(options->mainwindow, NULL, mouseButton, xroot, yroot, dragTime); + GdkSurface *surface = gtk_native_get_surface(GTK_NATIVE(options->mainwindow)); + gdk_toplevel_begin_move(GDK_TOPLEVEL(surface), NULL, mouseButton, xroot, yroot, dragTime); @@ - gdk_toplevel_begin_resize(options->mainwindow, options->edge, NULL, mouseButton, xroot, yroot, dragTime); + GdkSurface *surface = gtk_native_get_surface(GTK_NATIVE(options->mainwindow)); + gdk_toplevel_begin_resize(GDK_TOPLEVEL(surface), options->edge, NULL, mouseButton, xroot, yroot, dragTime);Also applies to: 663-667
v2/internal/frontend/desktop/linux/window_webkit6.go (3)
334-360: Wrap Run() GTK mutations on main thread and drop no-op Center().GTK widget mutations must run on GTK thread. Also Center() is a no-op in GTK4 path.
- if w.menubar != nil { - C.gtk_box_prepend(C.GTKBOX(unsafe.Pointer(w.vbox)), w.menubar) - } - - C.gtk_box_prepend(C.GTKBOX(unsafe.Pointer(w.webviewBox)), C.GTKWIDGET(w.webview)) - C.gtk_box_append(C.GTKBOX(unsafe.Pointer(w.vbox)), w.webviewBox) - - _url := C.CString(url) - defer C.free(unsafe.Pointer(_url)) - C.LoadIndex(w.webview, _url) + _url := C.CString(url) + defer C.free(unsafe.Pointer(_url)) + invokeOnMainThread(func() { + if w.menubar != nil { + C.gtk_box_prepend(C.GTKBOX(unsafe.Pointer(w.vbox)), w.menubar) + } + C.gtk_box_prepend(C.GTKBOX(unsafe.Pointer(w.webviewBox)), C.GTKWIDGET(w.webview)) + C.gtk_box_append(C.GTKBOX(unsafe.Pointer(w.vbox)), w.webviewBox) + C.LoadIndex(w.webview, _url) + C.gtk_window_present(w.asGTKWindow()) + }) if w.appoptions.StartHidden { w.Hide() } - C.gtk_window_present(w.asGTKWindow()) - - w.Center() switch w.appoptions.WindowStartState {
501-511: GTK dialog called from goroutine; dispatch to main thread.This is unsafe for GTK. Use
invokeOnMainThread.- go func() { - data := C.MessageDialogOptions{ - title: C.CString(title), - message: C.CString(message), - messageType: C.int(1), - } - - C.MessageDialog(unsafe.Pointer(&data)) - }() + go func() { + data := C.MessageDialogOptions{ + title: C.CString(title), + message: C.CString(message), + messageType: C.int(1), + } + invokeOnMainThread(func() { C.MessageDialog(unsafe.Pointer(&data)) }) + }()
182-184: Incorrect cast: asGTKBox must return the box widget, not the window.Casting
w.gtkWindowtoGtkBoxis invalid and may crash if used.func (w *Window) asGTKBox() *C.GtkBox { - return C.GTKBOX(w.gtkWindow) + return C.GTKBOX(unsafe.Pointer(w.vbox)) }
🧹 Nitpick comments (6)
v2/internal/frontend/desktop/linux/window_webkit6.go (4)
133-135: Remove unused variable.
buttonPressedNameis allocated and freed but never used.- buttonPressedName := C.CString("button-press-event") - defer C.free(unsafe.Pointer(buttonPressedName)) - C.ConnectButtons(unsafe.Pointer(webview)) + C.ConnectButtons(unsafe.Pointer(webview))
362-365: SetKeepAbove is a stub; document or feature-flag it.It does nothing on GTK4. Consider documenting the limitation or guarding the API to avoid misleading callers.
371-377: Prefergtk_window_set_default_sizeOR provide a resize API; current SetSize duplicates SetDefaultSize.If intentional, comment why. Otherwise, expose a
Resizethat changes current size.
222-232: Size() returns default size, not current size.If the intent is current geometry, consider using a GTK4-appropriate getter (or document this).
v2/internal/frontend/desktop/linux/window_webkit6.c (2)
187-195: Do not unref the CSS provider if you reuse it later.You store
windowCssProviderglobally and callload_from_datalater; keep a strong ref.- windowCssProvider = gtk_css_provider_new(); - gtk_style_context_add_provider( + windowCssProvider = gtk_css_provider_new(); + gtk_style_context_add_provider( gtk_widget_get_style_context(GTK_WIDGET(options->webviewBox)), GTK_STYLE_PROVIDER(windowCssProvider), GTK_STYLE_PROVIDER_PRIORITY_USER); - g_object_unref(windowCssProvider); + // keep our reference; free when tearing down the window if neededAlso applies to: 197-204
551-552: Remove unused stub.
onDeleteis defined but unused.-static void onDelete(GtkWidget* self) {} +// (removed unused stub)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v2/internal/frontend/desktop/linux/window_webkit6.c(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
v2/internal/frontend/desktop/linux/window_webkit6.go (5)
v2/internal/frontend/desktop/linux/window.go (2)
NewWindow(62-149)Window(40-53)v2/internal/frontend/desktop/linux/window_webkit6.c (36)
SetupInvokeSignal(125-128)SetupWebview(554-605)ConnectButtons(351-361)DevtoolsEnabled(607-617)InstallF12Hotkey(915-925)DisableContextMenu(300-304)SetBackgroundColour(157-204)SetTitle(216-222)SetWindowIcon(131-147)SetWindowTransparency(149-153)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)Fullscreen(455-471)ExecuteOnMainThread(25-28)UnFullscreen(473-478)Center(389-411)SetPosition(237-249)SetMinMaxSize(252-291)Show(413-418)Hide(420-425)Maximise(427-432)UnMaximise(434-439)Minimise(441-446)UnMinimise(448-453)IsFullscreen(363-367)IsMaximised(369-373)IsMinimised(375-385)LoadIndex(619-622)ExecuteJS(678-684)StartDrag(643-649)StartResize(669-676)AllocFileFilterArray(756-759)Opendialog(809-896)MessageDialog(704-752)ShowInspector(905-908)v2/pkg/options/linux/linux.go (2)
WebviewGpuPolicy(4-4)WebviewGpuPolicyNever(12-12)v2/pkg/options/options.go (5)
DragAndDrop(201-216)RGBA(111-116)WindowStartState(22-22)Minimised(27-27)Maximised(26-26)v2/internal/frontend/desktop/linux/calloc.go (1)
NewCalloc(18-20)
v2/internal/frontend/desktop/linux/window_webkit6.c (3)
v2/internal/frontend/runtime/desktop/events.js (1)
data(105-105)v2/internal/frontend/runtime/desktop/draganddrop.js (1)
files(136-136)v2/internal/frontend/desktop/linux/dialog.go (1)
GTK_FILE_CHOOSER_ACTION_SAVE(19-19)
v2/internal/frontend/desktop/linux/window_webkit6.h (1)
v2/internal/frontend/desktop/linux/window_webkit6.c (39)
ExecuteOnMainThread(25-28)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)SetupInvokeSignal(125-128)SetWindowIcon(131-147)SetWindowTransparency(149-153)SetBackgroundColour(157-204)SetTitle(216-222)SetPosition(237-249)SetMinMaxSize(252-291)DisableContextMenu(300-304)ConnectButtons(351-361)IsFullscreen(363-367)IsMaximised(369-373)IsMinimised(375-385)Center(389-411)Show(413-418)Hide(420-425)Maximise(427-432)UnMaximise(434-439)Minimise(441-446)UnMinimise(448-453)Fullscreen(455-471)UnFullscreen(473-478)SetupWebview(554-605)LoadIndex(619-622)DevtoolsEnabled(607-617)ExecuteJS(678-684)StartDrag(643-649)StartResize(669-676)MessageDialog(704-752)AllocFileFilterArray(756-759)Opendialog(809-896)sendShowInspectorMessage(910-912)ShowInspector(905-908)InstallF12Hotkey(915-925)createApp(933-937)runApp(939-942)
🪛 Clang (14.0.6)
v2/internal/frontend/desktop/linux/window_webkit6.c
[error] 4-4: 'jsc/jsc.h' file not found
(clang-diagnostic-error)
v2/internal/frontend/desktop/linux/window_webkit6.h
[error] 7-7: 'jsc/jsc.h' file not found
(clang-diagnostic-error)
🔇 Additional comments (1)
v2/internal/frontend/desktop/linux/window_webkit6.go (1)
6-12: Based on my research, the original JavaScriptCore API (e.g. JSContextRef and JSObjectRef) has been replaced by the GObject-style JavaScriptCore API (e.g. [[email protected]] and [[email protected]]) that is available since 2.22, and this is available in javascriptcoregtk-6.0. Additionally, the Ubuntu bug tracker shows thatjsc/jsc.h: No such file or directoryerrors occur when the header is not properly included, and multiple real-world examples demonstrate that projects building with WebKit report missing javascriptcoregtk as a separate dependency requirement.The review comment is technically sound: the code includes
<jsc/jsc.h>, which requires a separate pkg-config declaration forjavascriptcoregtk-6.0. The suggested diff is correct.Add missing pkg-config for JavaScriptCore to fix 'jsc/jsc.h' include.
The JavaScriptCore 6.0 API is a separate package that must be declared in pkg-config flags. Without
javascriptcoregtk-6.0, compilation will fail.Apply:
/* -#cgo pkg-config: gtk4 webkitgtk-6.0 +#cgo pkg-config: gtk4 webkitgtk-6.0 javascriptcoregtk-6.0
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
v2/internal/frontend/desktop/linux/window_webkit6.go (5)
350-351: Center() is a no‑op on GTK4; drop the call or annotate.Avoid misleading no‑op calls until implemented.
Apply this diff:
- w.Center() + // Center() is currently a no-op on GTK4/webkit_6.
327-331: SetWindowIcon currently calls a stub.No icon will be set under webkit_6. Either implement in C or gate this call behind availability and log a single warning.
181-183: Fix incorrect cast in asGTKBox().Casting GtkWindow to GtkBox is invalid; return the actual box.
Apply this diff:
func (w *Window) asGTKBox() *C.GtkBox { - return C.GTKBOX(w.gtkWindow) + return C.GTKBOX(unsafe.Pointer(w.vbox)) }
403-412: Quit() is a no‑op on GTK4; call GApplication quit.When mainLoop is nil, the app never exits. Keep the guard and also quit the GApplication on the GTK thread.
Apply this diff:
func (w *Window) Quit() { - if mainLoop == nil { - return - } - - C.g_main_loop_quit(mainLoop) - C.g_main_loop_unref(mainLoop) - - mainLoop = nil + if mainLoop != nil { + C.g_main_loop_quit(mainLoop) + C.g_main_loop_unref(mainLoop) + mainLoop = nil + } + invokeOnMainThread(func() { + C.g_application_quit((*C.GApplication)(unsafe.Pointer(w.gtkApp))) + }) }
500-514: Call MessageDialog on the GTK main thread (no goroutine).GTK APIs must run on the main thread.
Apply this diff:
func showModalDialogAndExit(title, message string) { - go func() { - data := C.MessageDialogOptions{ - title: C.CString(title), - message: C.CString(message), - messageType: C.int(1), - } - - C.MessageDialog(unsafe.Pointer(&data)) - }() + invokeOnMainThread(func() { + data := C.MessageDialogOptions{ + title: C.CString(title), + message: C.CString(message), + messageType: C.int(1), + } + C.MessageDialog(unsafe.Pointer(&data)) + }) <-messageDialogResult log.Fatal(message) }v2/internal/frontend/desktop/linux/window_webkit6.c (4)
393-416: Center() is intentionally a no‑op (GTK4 removed move).Either implement a GTK4 alternative later or keep as documented no‑op. Ensure the Go call site doesn’t invoke it (see window_webkit6.go Lines 350‑351).
642-647: Fix type passed to gdk_toplevel_begin_move.First arg must be GdkToplevel*. Convert from GtkWindow via GtkNative/GdkSurface.
Apply this diff:
- gdk_toplevel_begin_move(options->mainwindow, NULL, mouseButton, xroot, yroot, dragTime); + GdkSurface *surface = gtk_native_get_surface(GTK_NATIVE(options->mainwindow)); + gdk_toplevel_begin_move(GDK_TOPLEVEL(surface), NULL, mouseButton, xroot, yroot, dragTime);
669-673: Fix type passed to gdk_toplevel_begin_resize.Same conversion needed as for move.
Apply this diff:
- gdk_toplevel_begin_resize(options->mainwindow, options->edge, NULL, mouseButton, xroot, yroot, dragTime); + GdkSurface *surface = gtk_native_get_surface(GTK_NATIVE(options->mainwindow)); + gdk_toplevel_begin_resize(GDK_TOPLEVEL(surface), options->edge, NULL, mouseButton, xroot, yroot, dragTime);
760-760: Fix invalid C prototype order (“void extern”).C requires “extern void …”.
Apply this diff:
-void extern processOpenFileResult(void *); +extern void processOpenFileResult(void *);
🧹 Nitpick comments (2)
v2/internal/frontend/desktop/linux/window_webkit6.go (1)
6-9: Surface required system packages in “doctor” for -tags webkit_6.Add doctor checks to verify gtk4 and webkitgtk-6.0 dev packages and headers (e.g., jsc/jsc.h). This will preempt build failures.
v2/internal/frontend/desktop/linux/window_webkit6.h (1)
7-9: Build deps: ensure jsc/jsc.h is available.Document that webkitgtk-6.0 (packages providing JavaScriptCore headers) must be installed; consider adding a doctor check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v2/internal/frontend/desktop/linux/window_webkit6.c(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.go(1 hunks)v2/internal/frontend/desktop/linux/window_webkit6.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
v2/internal/frontend/desktop/linux/window_webkit6.go (4)
v2/internal/frontend/desktop/linux/window.go (2)
NewWindow(62-149)Window(40-53)v2/internal/frontend/desktop/linux/window_webkit6.c (36)
SetupInvokeSignal(130-133)SetupWebview(560-611)ConnectButtons(356-366)DevtoolsEnabled(613-623)InstallF12Hotkey(921-931)DisableContextMenu(305-309)SetBackgroundColour(162-209)SetTitle(221-227)SetWindowIcon(136-152)SetWindowTransparency(154-158)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)Fullscreen(460-476)ExecuteOnMainThread(25-28)UnFullscreen(478-483)Center(394-416)SetPosition(242-254)SetMinMaxSize(257-296)Show(418-423)Hide(425-430)Maximise(432-437)UnMaximise(439-444)Minimise(446-451)UnMinimise(453-458)IsFullscreen(368-372)IsMaximised(374-378)IsMinimised(380-390)LoadIndex(625-628)ExecuteJS(684-690)StartDrag(649-655)StartResize(675-682)AllocFileFilterArray(762-765)Opendialog(815-902)MessageDialog(710-758)ShowInspector(911-914)v2/pkg/options/linux/linux.go (2)
WebviewGpuPolicy(4-4)WebviewGpuPolicyNever(12-12)v2/internal/frontend/desktop/linux/calloc.go (1)
NewCalloc(18-20)
v2/internal/frontend/desktop/linux/window_webkit6.c (2)
v2/internal/frontend/runtime/desktop/draganddrop.js (1)
files(136-136)v2/internal/frontend/desktop/linux/dialog.go (1)
GTK_FILE_CHOOSER_ACTION_SAVE(19-19)
v2/internal/frontend/desktop/linux/window_webkit6.h (1)
v2/internal/frontend/desktop/linux/window_webkit6.c (39)
ExecuteOnMainThread(25-28)GTKWIDGET(30-33)GTKWINDOW(35-38)GTKBOX(40-43)SetupInvokeSignal(130-133)SetWindowIcon(136-152)SetWindowTransparency(154-158)SetBackgroundColour(162-209)SetTitle(221-227)SetPosition(242-254)SetMinMaxSize(257-296)DisableContextMenu(305-309)ConnectButtons(356-366)IsFullscreen(368-372)IsMaximised(374-378)IsMinimised(380-390)Center(394-416)Show(418-423)Hide(425-430)Maximise(432-437)UnMaximise(439-444)Minimise(446-451)UnMinimise(453-458)Fullscreen(460-476)UnFullscreen(478-483)SetupWebview(560-611)LoadIndex(625-628)DevtoolsEnabled(613-623)ExecuteJS(684-690)StartDrag(649-655)StartResize(675-682)MessageDialog(710-758)AllocFileFilterArray(762-765)Opendialog(815-902)sendShowInspectorMessage(916-918)ShowInspector(911-914)InstallF12Hotkey(921-931)createApp(939-943)runApp(945-948)
🪛 Clang (14.0.6)
v2/internal/frontend/desktop/linux/window_webkit6.c
[error] 4-4: 'jsc/jsc.h' file not found
(clang-diagnostic-error)
v2/internal/frontend/desktop/linux/window_webkit6.h
[error] 7-7: 'jsc/jsc.h' file not found
(clang-diagnostic-error)
| func (w *Window) GetPosition() (int, int) { | ||
| //// TODO: gtk_window_get_position was removed in gtk4 | ||
| // var width, height C.int | ||
| // var wg sync.WaitGroup | ||
| // wg.Add(1) | ||
| // invokeOnMainThread(func() { | ||
| // C.gtk_window_get_position(w.asGTKWindow(), &width, &height) | ||
| // wg.Done() | ||
| // }) | ||
| // wg.Wait() | ||
| // return int(width), int(height) | ||
| return 0, 0 | ||
| } |
There was a problem hiding this comment.
Position/min/max size are stubs; options are silently ignored.
GetPosition returns (0,0) and SetMin/MaxSize call a no-op C function. Either implement a GTK4-friendly fallback (size-allocate clamping) or clearly document the limitation and log once at startup.
Would you like a follow-up patch that clamps sizes via size-allocate and signals?
Also applies to: 247-261
🤖 Prompt for AI Agents
In v2/internal/frontend/desktop/linux/window_webkit6.go around lines 233-245
(and similarly for 247-261), GetPosition currently returns a hardcoded (0,0) and
SetMin/SetMaxSize are no-ops which silently ignore limits; replace these stubs
with a GTK4-compatible implementation or add a clear one-time startup log and
documented limitation: implement size clamping by connecting to the
"size-allocate" signal on the GTK window (on the main thread) to enforce min/max
constraints and maintain/return the last known position from allocate/configure
events, or if you cannot implement clamping now, add a single startup log
warning explaining that position/min/max are not supported on GTK4 and ensure
the functions return deterministic values and do not silently ignore inputs.
|
|
@leaanthony I don't see any workarounds for gtk4's removal of window positioning and I'll respect that that's the direction they want to go in so I've just left it as is. I've fixed checking window minimization state. Drag and drop should be working now. And I added window transparency but just be aware that the transparency affects the entire widget including the menu and all of the webview's contents. I'm marking this as ready. I've done all I can and it does successfully utilize webkitgtk-6. |
|
@leaanthony Are you able to look this over? |
3 participants
Description
This adds an option to run or build an app on Linux using webkitgtk-6.0.
Depends on:
Fixes #3193
Type of change
Important
Features which have been removed/deprecated from webkit or gtk4 have been marked with todos.
If they can be reimplemented, using this library option could potentially match the functionality of the current implementation.
How Has This Been Tested?
Ubuntu 22.04
The performance is usually noticeable on an app with scrolled content or a complex layout.
Running/Building the app with this PR's changes requires:
-tags webkit_6to the WailsdevorbuildcommandWarnings:
New application windows must be added after the GApplication::startup signal has been emitted.Test Configuration
Requires webkitgtk-6 and gtk4.
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
Improvements