From d9dfd57da5ebc4f8de0ad402ac484d92e888d85d Mon Sep 17 00:00:00 2001 From: Ara Date: Thu, 26 Jun 2025 05:11:25 +0530 Subject: [PATCH] Settings fix (#4405) Co-authored-by: Cline Evaluation --- .../settings/BrowserSettingsSection.tsx | 228 ++++++------------ .../src/components/settings/SettingsView.tsx | 35 ++- .../src/context/ExtensionStateContext.tsx | 8 +- 3 files changed, 108 insertions(+), 163 deletions(-) diff --git a/webview-ui/src/components/settings/BrowserSettingsSection.tsx b/webview-ui/src/components/settings/BrowserSettingsSection.tsx index 3af9a45b84..2792d20bec 100644 --- a/webview-ui/src/components/settings/BrowserSettingsSection.tsx +++ b/webview-ui/src/components/settings/BrowserSettingsSection.tsx @@ -1,5 +1,3 @@ -import { UpdateBrowserSettingsRequest } from "@shared/proto/browser" -import { EmptyRequest, StringRequest } from "@shared/proto/common" import { VSCodeButton, VSCodeCheckbox, VSCodeDropdown, VSCodeOption, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" import debounce from "debounce" import React, { useCallback, useEffect, useState } from "react" @@ -7,6 +5,13 @@ import styled from "styled-components" import { BROWSER_VIEWPORT_PRESETS } from "../../../../src/shared/BrowserSettings" import { useExtensionState } from "../../context/ExtensionStateContext" import { BrowserServiceClient } from "../../services/grpc-client" +import { EmptyRequest, StringRequest } from "@shared/proto/common" +import { BrowserSettings } from "@shared/BrowserSettings" + +interface BrowserSettingsSectionProps { + localBrowserSettings: BrowserSettings + onBrowserSettingsChange: (settings: BrowserSettings) => void +} const ConnectionStatusIndicator = ({ isChecking, @@ -51,9 +56,12 @@ const CollapsibleContent = styled.div<{ isOpen: boolean }>` visibility: ${({ isOpen }) => (isOpen ? "visible" : "hidden")}; ` -export const BrowserSettingsSection: React.FC = () => { +export const BrowserSettingsSection: React.FC = ({ + localBrowserSettings, + onBrowserSettingsChange, +}) => { const { browserSettings } = useExtensionState() - const [localChromePath, setLocalChromePath] = useState(browserSettings.chromeExecutablePath || "") + const [localChromePath, setLocalChromePath] = useState(localBrowserSettings.chromeExecutablePath || "") const [isCheckingConnection, setIsCheckingConnection] = useState(false) const [connectionStatus, setConnectionStatus] = useState(null) const [relaunchResult, setRelaunchResult] = useState<{ success: boolean; message: string } | null>(null) @@ -86,23 +94,24 @@ export const BrowserSettingsSection: React.FC = () => { }) }, []) - // Sync localChromePath with global state + // Sync localChromePath with prop changes useEffect(() => { - if (browserSettings.chromeExecutablePath !== localChromePath) { - setLocalChromePath(browserSettings.chromeExecutablePath || "") + if (localBrowserSettings.chromeExecutablePath !== localChromePath) { + setLocalChromePath(localBrowserSettings.chromeExecutablePath || "") } - // Removed sync for local disableToolUse state - }, [browserSettings.chromeExecutablePath, browserSettings.disableToolUse]) + }, [localBrowserSettings.chromeExecutablePath]) // Debounced connection check function const debouncedCheckConnection = useCallback( debounce(() => { - if (browserSettings.remoteBrowserEnabled) { + if (localBrowserSettings.remoteBrowserEnabled) { setIsCheckingConnection(true) setConnectionStatus(null) - if (browserSettings.remoteBrowserHost) { + if (localBrowserSettings.remoteBrowserHost) { // Use gRPC for testBrowserConnection - BrowserServiceClient.testBrowserConnection(StringRequest.create({ value: browserSettings.remoteBrowserHost })) + BrowserServiceClient.testBrowserConnection( + StringRequest.create({ value: localBrowserSettings.remoteBrowserHost }), + ) .then((result) => { setConnectionStatus(result.success) setIsCheckingConnection(false) @@ -126,153 +135,70 @@ export const BrowserSettingsSection: React.FC = () => { } } }, 1000), - [browserSettings.remoteBrowserEnabled, browserSettings.remoteBrowserHost], + [localBrowserSettings.remoteBrowserEnabled, localBrowserSettings.remoteBrowserHost], ) // Check connection when component mounts or when remote settings change useEffect(() => { - if (browserSettings.remoteBrowserEnabled) { + if (localBrowserSettings.remoteBrowserEnabled) { debouncedCheckConnection() } else { setConnectionStatus(null) } - }, [browserSettings.remoteBrowserEnabled, browserSettings.remoteBrowserHost, debouncedCheckConnection]) + }, [localBrowserSettings.remoteBrowserEnabled, localBrowserSettings.remoteBrowserHost, debouncedCheckConnection]) const handleViewportChange = (event: Event) => { const target = event.target as HTMLSelectElement const selectedSize = BROWSER_VIEWPORT_PRESETS[target.value as keyof typeof BROWSER_VIEWPORT_PRESETS] if (selectedSize) { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: selectedSize.width, - height: selectedSize.height, - }, - remoteBrowserEnabled: browserSettings.remoteBrowserEnabled, - remoteBrowserHost: browserSettings.remoteBrowserHost, - chromeExecutablePath: browserSettings.chromeExecutablePath, - disableToolUse: browserSettings.disableToolUse, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update browser settings") - } - }) - .catch((error) => { - console.error("Error updating browser settings:", error) - }) + onBrowserSettingsChange({ + ...localBrowserSettings, + viewport: { + width: selectedSize.width, + height: selectedSize.height, + }, + }) } } const updateRemoteBrowserEnabled = (enabled: boolean) => { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: browserSettings.viewport.width, - height: browserSettings.viewport.height, - }, - remoteBrowserEnabled: enabled, - // If disabling, also clear the host - remoteBrowserHost: enabled ? browserSettings.remoteBrowserHost : undefined, - chromeExecutablePath: browserSettings.chromeExecutablePath, - disableToolUse: browserSettings.disableToolUse, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update browser settings") - } - }) - .catch((error) => { - console.error("Error updating browser settings:", error) - }) + onBrowserSettingsChange({ + ...localBrowserSettings, + remoteBrowserEnabled: enabled, + // If disabling, also clear the host + remoteBrowserHost: enabled ? localBrowserSettings.remoteBrowserHost : undefined, + }) } const updateRemoteBrowserHost = (host: string | undefined) => { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: browserSettings.viewport.width, - height: browserSettings.viewport.height, - }, - remoteBrowserEnabled: browserSettings.remoteBrowserEnabled, - remoteBrowserHost: host, - chromeExecutablePath: browserSettings.chromeExecutablePath, - disableToolUse: browserSettings.disableToolUse, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update browser settings") - } - }) - .catch((error) => { - console.error("Error updating browser settings:", error) - }) + onBrowserSettingsChange({ + ...localBrowserSettings, + remoteBrowserHost: host, + }) } const debouncedUpdateChromePath = useCallback( debounce((newPath: string | undefined) => { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: browserSettings.viewport.width, - height: browserSettings.viewport.height, - }, - remoteBrowserEnabled: browserSettings.remoteBrowserEnabled, - remoteBrowserHost: browserSettings.remoteBrowserHost, - chromeExecutablePath: newPath, - disableToolUse: browserSettings.disableToolUse, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update browser settings for chromeExecutablePath") - } - }) - .catch((error) => { - console.error("Error updating browser settings for chromeExecutablePath:", error) - }) + onBrowserSettingsChange({ + ...localBrowserSettings, + chromeExecutablePath: newPath, + }) }, 500), - [browserSettings], + [localBrowserSettings, onBrowserSettingsChange], ) const updateChromeExecutablePath = (path: string | undefined) => { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: browserSettings.viewport.width, - height: browserSettings.viewport.height, - }, - remoteBrowserEnabled: browserSettings.remoteBrowserEnabled, - remoteBrowserHost: browserSettings.remoteBrowserHost, - chromeExecutablePath: path, - disableToolUse: browserSettings.disableToolUse, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update browser settings") - } - }) - .catch((error) => { - console.error("Error updating browser settings:", error) - }) + setLocalChromePath(path || "") + debouncedUpdateChromePath(path) } // Function to check connection once without changing UI state immediately const checkConnectionOnce = useCallback(() => { // Don't show the spinner for every check to avoid UI flicker // We'll rely on the response to update the connectionStatus - if (browserSettings.remoteBrowserHost) { + if (localBrowserSettings.remoteBrowserHost) { // Use gRPC for testBrowserConnection - BrowserServiceClient.testBrowserConnection(StringRequest.create({ value: browserSettings.remoteBrowserHost })) + BrowserServiceClient.testBrowserConnection(StringRequest.create({ value: localBrowserSettings.remoteBrowserHost })) .then((result) => { setConnectionStatus(result.success) }) @@ -290,12 +216,12 @@ export const BrowserSettingsSection: React.FC = () => { setConnectionStatus(false) }) } - }, [browserSettings.remoteBrowserHost]) + }, [localBrowserSettings.remoteBrowserHost]) // Setup continuous polling for connection status when remote browser is enabled useEffect(() => { // Only poll if remote browser mode is enabled - if (!browserSettings.remoteBrowserEnabled) { + if (!localBrowserSettings.remoteBrowserEnabled) { // Make sure we're not showing checking state when disabled setIsCheckingConnection(false) return @@ -311,30 +237,13 @@ export const BrowserSettingsSection: React.FC = () => { // Cleanup the interval if the component unmounts or remote browser is disabled return () => clearInterval(pollInterval) - }, [browserSettings.remoteBrowserEnabled, checkConnectionOnce]) + }, [localBrowserSettings.remoteBrowserEnabled, checkConnectionOnce]) const updateDisableToolUse = (disabled: boolean) => { - BrowserServiceClient.updateBrowserSettings( - UpdateBrowserSettingsRequest.create({ - metadata: {}, - viewport: { - width: browserSettings.viewport.width, - height: browserSettings.viewport.height, - }, - remoteBrowserEnabled: browserSettings.remoteBrowserEnabled, - remoteBrowserHost: browserSettings.remoteBrowserHost, - chromeExecutablePath: browserSettings.chromeExecutablePath, - disableToolUse: disabled, - }), - ) - .then((response) => { - if (!response.value) { - console.error("Failed to update disableToolUse setting") - } - }) - .catch((error) => { - console.error("Error updating disableToolUse setting:", error) - }) + onBrowserSettingsChange({ + ...localBrowserSettings, + disableToolUse: disabled, + }) } const relaunchChromeDebugMode = () => { @@ -361,16 +270,16 @@ export const BrowserSettingsSection: React.FC = () => { } // Determine if we should show the relaunch button - const isRemoteEnabled = Boolean(browserSettings.remoteBrowserEnabled) + const isRemoteEnabled = Boolean(localBrowserSettings.remoteBrowserEnabled) const shouldShowRelaunchButton = isRemoteEnabled && connectionStatus === false - const isSubSettingsOpen = !(browserSettings.disableToolUse || false) + const isSubSettingsOpen = !(localBrowserSettings.disableToolUse || false) return (
{/* Master Toggle */}
updateDisableToolUse((e.target as HTMLInputElement).checked)}> Disable browser tool usage @@ -394,8 +303,8 @@ export const BrowserSettingsSection: React.FC = () => { Object.entries(BROWSER_VIEWPORT_PRESETS).find(([_, size]) => { const typedSize = size as { width: number; height: number } return ( - typedSize.width === browserSettings.viewport.width && - typedSize.height === browserSettings.viewport.height + typedSize.width === localBrowserSettings.viewport.width && + typedSize.height === localBrowserSettings.viewport.height ) })?.[0] } @@ -422,14 +331,14 @@ export const BrowserSettingsSection: React.FC = () => { {/* This div now contains Remote Connection & Chrome Path */}
updateRemoteBrowserEnabled((e.target as HTMLInputElement).checked)}> Use remote browser connection

{ Enable Cline to use your Chrome {isBundled ? "(not detected on your machine)" : detectedChromePath ? ` (${detectedChromePath})` : ""}. You can specify a custom path below. Using a remote browser connection requires starting Chrome in debug mode - {browserSettings.remoteBrowserEnabled ? ( + {localBrowserSettings.remoteBrowserEnabled ? ( <> {" "} manually (--remote-debugging-port=9222) or using the button below. Enter the host @@ -452,10 +361,10 @@ export const BrowserSettingsSection: React.FC = () => { )}

{/* Moved remote-specific settings to appear directly after enabling remote connection */} - {browserSettings.remoteBrowserEnabled && ( + {localBrowserSettings.remoteBrowserEnabled && (
updateRemoteBrowserHost(e.target.value || undefined)} @@ -507,8 +416,7 @@ export const BrowserSettingsSection: React.FC = () => { style={{ width: "100%" }} onChange={(e: any) => { const newValue = e.target.value || "" - setLocalChromePath(newValue) - debouncedUpdateChromePath(newValue) // Send "" if empty, not undefined + updateChromeExecutablePath(newValue) }} />

{ mcpResponsesCollapsed, setMcpResponsesCollapsed, setApiConfiguration, + browserSettings, } = useExtensionState() + // Local state for browser settings + const [localBrowserSettings, setLocalBrowserSettings] = useState(browserSettings) + // Store the original state to detect changes const originalState = useRef({ apiConfiguration, @@ -154,6 +159,7 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { terminalReuseEnabled, terminalOutputLineLimit, defaultTerminalProfile, + browserSettings, }) const [apiErrorMessage, setApiErrorMessage] = useState(undefined) const [modelIdErrorMessage, setModelIdErrorMessage] = useState(undefined) @@ -207,6 +213,23 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { } as StringRequest) } + // Update browser settings if they have changed + if (JSON.stringify(localBrowserSettings) !== JSON.stringify(originalState.current.browserSettings)) { + const { BrowserServiceClient } = await import("@/services/grpc-client") + const { UpdateBrowserSettingsRequest } = await import("@shared/proto/browser") + + await BrowserServiceClient.updateBrowserSettings( + UpdateBrowserSettingsRequest.create({ + metadata: {}, + viewport: localBrowserSettings.viewport, + remoteBrowserEnabled: localBrowserSettings.remoteBrowserEnabled, + remoteBrowserHost: localBrowserSettings.remoteBrowserHost, + chromeExecutablePath: localBrowserSettings.chromeExecutablePath, + disableToolUse: localBrowserSettings.disableToolUse, + }), + ) + } + // Update the original state to reflect the saved changes originalState.current = { apiConfiguration, @@ -221,6 +244,7 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { terminalReuseEnabled, terminalOutputLineLimit, defaultTerminalProfile, + browserSettings: localBrowserSettings, } } catch (error) { console.error("Failed to update settings:", error) @@ -251,7 +275,8 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { shellIntegrationTimeout !== originalState.current.shellIntegrationTimeout || terminalOutputLineLimit !== originalState.current.terminalOutputLineLimit || terminalReuseEnabled !== originalState.current.terminalReuseEnabled || - defaultTerminalProfile !== originalState.current.defaultTerminalProfile + defaultTerminalProfile !== originalState.current.defaultTerminalProfile || + JSON.stringify(localBrowserSettings) !== JSON.stringify(originalState.current.browserSettings) setHasUnsavedChanges(hasChanges) }, [ @@ -267,6 +292,7 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { terminalReuseEnabled, terminalOutputLineLimit, defaultTerminalProfile, + localBrowserSettings, ]) // Handle cancel button click @@ -319,6 +345,8 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => { if (typeof setMcpResponsesCollapsed === "function") { setMcpResponsesCollapsed(originalState.current.mcpResponsesCollapsed ?? false) } + // Reset browser settings + setLocalBrowserSettings(originalState.current.browserSettings) // Close settings view onDone() } @@ -689,7 +717,10 @@ const SettingsView = ({ onDone, targetSection }: SettingsViewProps) => {

{renderSectionHeader("browser")}
- +
)} diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index cb4caf93f2..c462183dfc 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -14,7 +14,7 @@ import { TerminalProfile } from "@shared/proto/state" import { convertProtoToClineMessage } from "@shared/proto-conversions/cline-message" import { convertProtoMcpServersToMcpServers } from "@shared/proto-conversions/mcp/mcp-server-conversion" import { DEFAULT_AUTO_APPROVAL_SETTINGS } from "@shared/AutoApprovalSettings" -import { DEFAULT_BROWSER_SETTINGS } from "@shared/BrowserSettings" +import { DEFAULT_BROWSER_SETTINGS, BrowserSettings } from "@shared/BrowserSettings" import { ChatSettings, DEFAULT_CHAT_SETTINGS } from "@shared/ChatSettings" import { DEFAULT_PLATFORM, ExtensionMessage, ExtensionState } from "@shared/ExtensionMessage" import { TelemetrySetting } from "@shared/TelemetrySetting" @@ -79,6 +79,7 @@ interface ExtensionStateContextType extends ExtensionState { setMcpMarketplaceCatalog: (value: McpMarketplaceCatalog) => void setTotalTasksSize: (value: number | null) => void setAvailableTerminalProfiles: (profiles: TerminalProfile[]) => void // Setter for profiles + setBrowserSettings: (value: BrowserSettings) => void // Refresh functions refreshOpenRouterModels: () => void @@ -838,6 +839,11 @@ export const ExtensionStateContextProvider: React.FC<{ refreshOpenRouterModels, onRelinquishControl, setUserInfo: (userInfo?: UserInfo) => setState((prevState) => ({ ...prevState, userInfo })), + setBrowserSettings: (value: BrowserSettings) => + setState((prevState) => ({ + ...prevState, + browserSettings: value, + })), } return {children}