Conversation
Three delivery bugs in View::ProcessMouseEvent, found via the F8 dev menu's teleport list: - WM_MOUSEWHEEL reports the cursor in screen coordinates (every other mouse message is client-relative) — convert with ScreenToClient before mapping into the view, so wheel events land on the element actually under the cursor when the client origin isn't at (0,0). - Events were sent with modifiers = 0; Blink sustains drags (scrollbar thumb, text selection) off the button flags carried by the move events, so every drag ended instantly. Forward the live MK_* button and Ctrl/Shift state from wParam (low word for the wheel message). - WM_LBUTTONDBLCLK was unhandled: the game window class has CS_DBLCLKS, so the second rapid click arrived as DBLCLK and was swallowed. Send it as a click_count=2 press so fast repeated clicks and page dblclick handlers work.
Walkthrough
ChangesMouse event translation
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/framework/src/gui/view.cpp (1)
208-215: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
click_countmismatch between double-click press and release.The
WM_LBUTTONDBLCLKhandler sends a press withclick_count=2, but the subsequentWM_LBUTTONUPhandler (line 214) always sendsclick_count=1. CEF/Blink expects the release event's click count to match the press event's click count for proper double-click recognition (e.g., double-click-to-select-word). Without a matching release, the double-click may not be reliably interpreted by the renderer.Track the current click count so the
WM_LBUTTONUPafter a double-click also sends2.🐛 Proposed fix: track click count for release events
void View::ProcessMouseEvent(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) { if (!_browser || !_shouldDisplay || !_hasFocus) { return; } auto host = _browser->GetHost(); static int clickCount = 1; // Handle mouse wheel separately. Unlike the client-relative messages below, // WM_MOUSEWHEEL reports the cursor in screen coordinates, and packs the // button/modifier state into the low word of wParam. if (msg == WM_MOUSEWHEEL) { POINT pt {GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)}; ::ScreenToClient(hWnd, &pt); CefMouseEvent cefEvent; cefEvent.x = pt.x - _x; cefEvent.y = pt.y - _y; cefEvent.modifiers = CefMouseModifiers(GET_KEYSTATE_WPARAM(wParam)); int delta = GET_WHEEL_DELTA_WPARAM(wParam); host->SendMouseWheelEvent(cefEvent, 0, delta); return; } CefMouseEvent cefEvent; cefEvent.x = GET_X_LPARAM(lParam) - _x; cefEvent.y = GET_Y_LPARAM(lParam) - _y; cefEvent.modifiers = CefMouseModifiers(wParam); switch (msg) { case WM_MOUSEMOVE: { _cursorPos = {cefEvent.x, cefEvent.y}; host->SendMouseMoveEvent(cefEvent, false); } break; case WM_LBUTTONDOWN: { _isMouseDown = true; clickCount = 1; host->SendMouseClickEvent(cefEvent, MBT_LEFT, false, clickCount); } break; case WM_LBUTTONDBLCLK: { _isMouseDown = true; clickCount = 2; host->SendMouseClickEvent(cefEvent, MBT_LEFT, false, clickCount); } break; case WM_LBUTTONUP: { _isMouseDown = false; host->SendMouseClickEvent(cefEvent, MBT_LEFT, true, clickCount); } break; case WM_RBUTTONDOWN: { host->SendMouseClickEvent(cefEvent, MBT_RIGHT, false, 1); } break; case WM_RBUTTONUP: { host->SendMouseClickEvent(cefEvent, MBT_RIGHT, true, 1); } break; case WM_MBUTTONDOWN: { host->SendMouseClickEvent(cefEvent, MBT_MIDDLE, false, 1); } break; case WM_MBUTTONUP: { host->SendMouseClickEvent(cefEvent, MBT_MIDDLE, true, 1); } break; } }Note: Using a
staticlocal is a minimal fix. For thread safety and cleanliness, prefer storingclickCountas a member variable (e.g.,_mouseClickCount) reset onWM_LBUTTONDOWN.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/framework/src/gui/view.cpp` around lines 208 - 215, Track the current mouse click count in the view state, resetting it on WM_LBUTTONDOWN and setting it to 2 on WM_LBUTTONDBLCLK; update the WM_LBUTTONUP handler to pass the stored count instead of always passing 1, so double-click press and release events match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@code/framework/src/gui/view.cpp`:
- Around line 208-215: Track the current mouse click count in the view state,
resetting it on WM_LBUTTONDOWN and setting it to 2 on WM_LBUTTONDBLCLK; update
the WM_LBUTTONUP handler to pass the stored count instead of always passing 1,
so double-click press and release events match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a246009-23be-414f-988f-baebd213d55e
📒 Files selected for processing (1)
code/framework/src/gui/view.cpp
2 participants
GUI Input for HL F8 mod window was stuck when click-dragging and using mouse scrollwheel in windowed mode. Also added handling for double click in case it's needed somewhere.
=====
Three delivery bugs in View::ProcessMouseEvent, found via the F8 dev menu's teleport list:
WM_MOUSEWHEEL reports the cursor in screen coordinates (every other mouse message is client-relative) — convert with ScreenToClient before mapping into the view, so wheel events land on the element actually under the cursor when the client origin isn't at (0,0).
Events were sent with modifiers = 0; Blink sustains drags (scrollbar thumb, text selection) off the button flags carried by the move events, so every drag ended instantly. Forward the live MK_* button and Ctrl/Shift state from wParam (low word for the wheel message).
WM_LBUTTONDBLCLK was unhandled: the game window class has CS_DBLCLKS, so the second rapid click arrived as DBLCLK and was swallowed. Send it as a click_count=2 press so fast repeated clicks and page dblclick handlers work.
Summary by CodeRabbit