Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chrome: Support top offset for dialogs/popups with frameless window #3628

Closed
magreenblatt opened this issue Jan 11, 2024 · 3 comments
Closed
Labels
chrome Related to the Chrome runtime enhancement Enhancement request

Comments

@magreenblatt
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
By default, Chrome/Chromium shows dialogs/popups immediately underneath the address bar. Frameless windows often do not have an address bar or title bar so dialogs/popups are shown aligned with the top of the window. It would be better to offset these windows either explicitly or implicitly, similar to #3461.

Describe the solution you'd like
Implement implicit offset logic similar to #3461.

Describe alternatives you've considered
Add a new CefWindowDelegate callback for retrieving the top window offset.

@magreenblatt magreenblatt added enhancement Enhancement request chrome Related to the Chrome runtime labels Jan 11, 2024
@magreenblatt
Copy link
Collaborator Author

magreenblatt commented Jan 11, 2024

Dialog position is retrieved via BrowserViewLayout::WebContentsModalDialogHostViews::GetDialogPosition.

For alert box (load https://tests/dialogs, click "Show Alert" button)

>	libcef.dll!BrowserViewLayout::WebContentsModalDialogHostViews::GetDialogPosition(const gfx::Size & size) Line 106	C++
 	libcef.dll!constrained_window::`anonymous namespace'::UpdateModalDialogPosition(views::Widget * widget, web_modal::ModalDialogHost * dialog_host, const gfx::Size & size) Line 132	C++
 	libcef.dll!constrained_window::UpdateWebContentsModalDialogPosition(views::Widget * widget, web_modal::WebContentsModalDialogHost * dialog_host) Line 185	C++
 	libcef.dll!constrained_window::NativeWebContentsModalDialogManagerViews::OnPositionRequiresUpdate() Line 158	C++
 	libcef.dll!constrained_window::NativeWebContentsModalDialogManagerViews::HostChanged(web_modal::WebContentsModalDialogHost * new_host) Line 209	C++
 	components_web_modal.dll!web_modal::WebContentsModalDialogManager::ShowDialogWithManager(aura::Window * dialog, std::__Cr::unique_ptr<web_modal::SingleWebContentsDialogManager,std::__Cr::default_delete<web_modal::SingleWebContentsDialogManager>> manager) Line 43	C++
 	libcef.dll!constrained_window::ShowModalDialog(aura::Window * dialog, content::WebContents * web_contents) Line 26	C++
 	libcef.dll!constrained_window::ShowWebModalDialogViews(views::WidgetDelegate * dialog, content::WebContents * initiator_web_contents) Line 216	C++
 	libcef.dll!JavaScriptTabModalDialogViewViews::JavaScriptTabModalDialogViewViews(content::WebContents * parent_web_contents, content::WebContents * alerting_web_contents, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & title, content::JavaScriptDialogType dialog_type, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & message_text, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & default_prompt_text, base::OnceCallback<void (bool, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> &)> dialog_callback, base::OnceCallback<void ()> dialog_force_closed_callback) Line 124	C++
 	libcef.dll!JavaScriptTabModalDialogManagerDelegateDesktop::CreateNewDialog(content::WebContents * alerting_web_contents, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & title, content::JavaScriptDialogType dialog_type, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & message_text, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & default_prompt_text, base::OnceCallback<void (bool, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> &)> dialog_callback, base::OnceCallback<void ()> dialog_force_closed_callback) Line 155	C++
 	libcef.dll!javascript_dialogs::TabModalDialogManager::RunJavaScriptDialog(content::WebContents * alerting_web_contents, content::RenderFrameHost * render_frame_host, content::JavaScriptDialogType dialog_type, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & message_text, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & default_prompt_text, base::OnceCallback<void (bool, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> &)> callback, bool * did_suppress_message) Line 161	C++
 	content.dll!content::WebContentsImpl::RunJavaScriptDialog(content::RenderFrameHostImpl * render_frame_host, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & message, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & default_prompt, content::JavaScriptDialogType dialog_type, bool disable_third_party_subframe_suppresion, base::OnceCallback<void (bool, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> &)> response_callback) Line 7663	C++
 	content.dll!content::RenderFrameHostImpl::RunJavaScriptDialog(const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & message, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & default_prompt, content::JavaScriptDialogType dialog_type, bool disable_third_party_subframe_suppresion, base::OnceCallback<void (bool, const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> &)> ipc_response_callback) Line 5634	C++
 	content.dll!content::RenderFrameHostImpl::RunModalAlertDialog(const std::__Cr::basic_string<char16_t,std::__Cr::char_traits<char16_t>,std::__Cr::allocator<char16_t>> & alert_message, bool disable_third_party_subframe_suppresion, base::OnceCallback<void ()> response_callback) Line 5579	C++

For modal dialog (select Tests > Dialog Window)

>	libcef.dll!BrowserViewLayout::WebContentsModalDialogHostViews::GetDialogPosition(const gfx::Size & size) Line 106	C++
	libcef.dll!constrained_window::`anonymous namespace'::UpdateModalDialogPosition(views::Widget * widget, web_modal::ModalDialogHost * dialog_host, const gfx::Size & size) Line 129	C++
 	libcef.dll!constrained_window::UpdateWebContentsModalDialogPosition(views::Widget * widget, web_modal::WebContentsModalDialogHost * dialog_host) Line 185	C++
 	libcef.dll!constrained_window::NativeWebContentsModalDialogManagerViews::OnPositionRequiresUpdate() Line 158	C++
 	libcef.dll!constrained_window::NativeWebContentsModalDialogManagerViews::HostChanged(web_modal::WebContentsModalDialogHost * new_host) Line 209	C++
 	components_web_modal.dll!web_modal::WebContentsModalDialogManager::ShowDialogWithManager(aura::Window * dialog, std::__Cr::unique_ptr<web_modal::SingleWebContentsDialogManager,std::__Cr::default_delete<web_modal::SingleWebContentsDialogManager>> manager) Line 43	C++
 	libcef.dll!constrained_window::ShowModalDialog(aura::Window * dialog, content::WebContents * web_contents) Line 26	C++
 	libcef.dll!CefWindowImpl::ShowAsBrowserModalDialog(scoped_refptr<CefBrowserView> browser_view) Line 147	C++
 	libcef.dll!`anonymous namespace'::window_show_as_browser_modal_dialog(_cef_window_t * self, _cef_browser_view_t * browser_view) Line 87	C++
 	cefclient.exe!CefWindowCToCpp::ShowAsBrowserModalDialog(scoped_refptr<CefBrowserView> browser_view) Line 89	C++
 	cefclient.exe!client::ViewsWindow::Show() Line 181	C++
 	cefclient.exe!client::ViewsWindow::OnWindowCreated(scoped_refptr<CefWindow> window) Line 738	C++

@magreenblatt
Copy link
Collaborator Author

The |browser_view_layout_->dialog_top_y_| value is configured in BrowserViewLayout::LayoutBookmarkAndInfoBars.

Running as: cefclient --enable-chrome-runtime --url=https://tests/dialogs --hide-frame --hide-controls

The |top| and |browser_view_y| arguments are both 0, so |dialog_top_y_| becomes -3 (kConstrainedWindowOverlap value)

>	libcef.dll!BrowserViewLayout::LayoutBookmarkAndInfoBars(int top, int browser_view_y) Line 610	C++
 	libcef.dll!BrowserViewLayout::Layout(views::View * browser_view) Line 408	C++
 	ui_views.dll!views::View::Layout() Line 873	C++
 	libcef.dll!BrowserView::Layout() Line 4249	C++
 	libcef.dll!CefViewView<BrowserView,CefBrowserViewDelegate>::Layout() Line 167	C++
 	libcef.dll!BrowserView::UpdateUIForContents(content::WebContents * contents) Line 4680	C++
 	libcef.dll!BrowserView::OnActiveTabChanged(content::WebContents * old_contents, content::WebContents * new_contents, int index, int reason) Line 1711	C++
 	libcef.dll!Browser::OnActiveTabChanged(content::WebContents * old_contents, content::WebContents * new_contents, int index, int reason) Line 2674	C++
 	libcef.dll!Browser::OnTabStripModelChanged(TabStripModel * tab_strip_model, const TabStripModelChange & change, const TabStripSelectionChange & selection) Line 1282	C++
 	libcef.dll!TabStripModel::OnChange(const TabStripModelChange & change, const TabStripSelectionChange & selection) Line 417	C++
 	libcef.dll!TabStripModel::InsertWebContentsAtImpl(int index, std::__Cr::unique_ptr<content::WebContents,std::__Cr::default_delete<content::WebContents>> contents, int add_types, std::__Cr::optional<tab_groups::TabGroupId> group) Line 1897	C++
 	libcef.dll!TabStripModel::AddWebContents(std::__Cr::unique_ptr<content::WebContents,std::__Cr::default_delete<content::WebContents>> contents, int index, ui::PageTransition transition, int add_types, std::__Cr::optional<tab_groups::TabGroupId> group) Line 993	C++
 	libcef.dll!Navigate(NavigateParams * params) Line 916	C++
 	libcef.dll!chrome::AddAndReturnTabAt(Browser * browser, const GURL & url, int idx, bool foreground, std::__Cr::optional<tab_groups::TabGroupId> group) Line 47	C++
 	libcef.dll!chrome::AddTabAt(Browser * browser, const GURL & url, int idx, bool foreground, std::__Cr::optional<tab_groups::TabGroupId> group) Line 64	C++
 	libcef.dll!ChromeBrowserHostImpl::Create(const CefBrowserCreateParams & params) Line 49	C++
 	libcef.dll!CefBrowserHostBase::Create(CefBrowserCreateParams & create_params) Line 169	C++
 	libcef.dll!CefBrowserViewImpl::OnBrowserViewAdded() Line 224	C++
 	libcef.dll!ChromeBrowserView::AddedToWidget() Line 57	C++

@magreenblatt
Copy link
Collaborator Author

The |top| and |browser_view_y| arguments are both 0

The |top| value passed to LayoutBookmarkAndInfoBars comes from BrowserFrame::GetTopInset. We don't want to change that value because it will offset the whole BrowserView downwards. Instead, we should add a new callback that directly modifies the value assigned to |dialog_top_y_| in LayoutBookmarkAndInfoBars.

magreenblatt added a commit that referenced this issue Jan 18, 2024
Dialogs will be excluded from regions near the top of the window
that contain overlays, draggable regions or titlebar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chrome Related to the Chrome runtime enhancement Enhancement request
Projects
None yet
Development

No branches or pull requests

1 participant