webium: get rid of extra titlebar and extra caption buttons on Mac [chromium/src : main]
Keren Zhu (Gerrit)
Keren Zhu added 1 comment![]( "Open in Gerrit")
Thank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keren Zhu (Gerrit)
Keren Zhu added 1 comment![]( "Open in Gerrit")
Thank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
fwiw the draggable area is determined by [`-[BridgedContentView hitTest:]`](https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/bridged_content_view.mm;l=289;drc=bc5784f03d2120209e60bc1c2ba266ecccb63dfb). The OS will call into this function on mouse events, and if the function returns nil, it tells the OS that the mouse is in a draggable point. Maybe either 1) somehow the function does not return nil?, or 2) the function still returns nil, but due to window style change, it is recognized as the draggable area?
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keren Zhu (Gerrit)
Keren Zhu added 1 comment![]( "Open in Gerrit")
Keren ZhuThank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
fwiw the draggable area is determined by [`-[BridgedContentView hitTest:]`](https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/bridged_content_view.mm;l=289;drc=bc5784f03d2120209e60bc1c2ba266ecccb63dfb). The OS will call into this function on mouse events, and if the function returns nil, it tells the OS that the mouse is in a draggable point. Maybe either 1) somehow the function does not return nil?, or 2) the function still returns nil, but due to window style change, it is recognized as the draggable area?
I debug and the issue is the missing CSS style `app-region: drag` on the titlebar area. Should be an easy fix. Can you look into that? Thanks!
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keren Zhu (Gerrit)
Keren Zhu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keren Zhu
| Code-Review | +1 |
1 comment
lgtm % window dragging. Thanks!
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Jensen (Gerrit)
Paul Jensen added 1 comment![]( "Open in Gerrit")
Keren ZhuThank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
Keren Zhufwiw the draggable area is determined by [`-[BridgedContentView hitTest:]`](https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/bridged_content_view.mm;l=289;drc=bc5784f03d2120209e60bc1c2ba266ecccb63dfb). The OS will call into this function on mouse events, and if the function returns nil, it tells the OS that the mouse is in a draggable point. Maybe either 1) somehow the function does not return nil?, or 2) the function still returns nil, but due to window style change, it is recognized as the draggable area?
I debug and the issue is the missing CSS style `app-region: drag` on the titlebar area. Should be an easy fix. Can you look into that? Thanks!
Hiding all of captionButtonsDiv was hiding all the draggable area. I changed to just hiding the buttons so the div is still there and draggable.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ken Buchanan (Gerrit)
Ken Buchanan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ken Buchanan
| Code-Review | +1 |
1 comment
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keren Zhu (Gerrit)
Keren Zhu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keren Zhu
| Code-Review | +1 |
Keren ZhuThank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
Keren Zhufwiw the draggable area is determined by [`-[BridgedContentView hitTest:]`](https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/bridged_content_view.mm;l=289;drc=bc5784f03d2120209e60bc1c2ba266ecccb63dfb). The OS will call into this function on mouse events, and if the function returns nil, it tells the OS that the mouse is in a draggable point. Maybe either 1) somehow the function does not return nil?, or 2) the function still returns nil, but due to window style change, it is recognized as the draggable area?
Paul JensenI debug and the issue is the missing CSS style `app-region: drag` on the titlebar area. Should be an easy fix. Can you look into that? Thanks!
Hiding all of captionButtonsDiv was hiding all the draggable area. I changed to just hiding the buttons so the div is still there and draggable.
The empty `.captionButtonsDiv` is a bit wired. Can we do 1) add `app-region: drag` to `.titlebarDiv`, 2) add `app-region: no-drag` to `.tabstripDiv`, and 3) remove `. captionButtonsDiv`?
Related details
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Jensen (Gerrit)
Paul Jensen added 1 comment![]( "Open in Gerrit")
Keren ZhuThank you for working on this. It seems that with this CL the tabstrip background area is no longer draggable.
Keren Zhufwiw the draggable area is determined by [`-[BridgedContentView hitTest:]`](https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/bridged_content_view.mm;l=289;drc=bc5784f03d2120209e60bc1c2ba266ecccb63dfb). The OS will call into this function on mouse events, and if the function returns nil, it tells the OS that the mouse is in a draggable point. Maybe either 1) somehow the function does not return nil?, or 2) the function still returns nil, but due to window style change, it is recognized as the draggable area?
Paul JensenI debug and the issue is the missing CSS style `app-region: drag` on the titlebar area. Should be an easy fix. Can you look into that? Thanks!
Keren ZhuHiding all of captionButtonsDiv was hiding all the draggable area. I changed to just hiding the buttons so the div is still there and draggable.
The empty `.captionButtonsDiv` is a bit wired. Can we do 1) add `app-region: drag` to `.titlebarDiv`, 2) add `app-region: no-drag` to `.tabstripDiv`, and 3) remove `. captionButtonsDiv`?
- Ken Buchanan
- Keren Zhu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keren Zhu (Gerrit)
Keren Zhu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keren Zhu
| Code-Review | +1 |
- Ken Buchanan
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ken Buchanan (Gerrit)
Ken Buchanan voted Code-Review+1![]( "Open in Gerrit")
- Paul Jensen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Jensen (Gerrit)
Paul Jensen voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Jensen (Gerrit)
Paul Jensen voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/resources/webui_browser/app.html.ts
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Change information
webium: get rid of extra titlebar and extra caption buttons on Mac
The extra titlebar is removed by changing window styles.
The extra caption buttons are hidden in favor of the native ones.
The tab strip is shifted over so as to not overlap the native
caption buttons.
- M chrome/browser/resources/webui_browser/app.css
- M chrome/browser/resources/webui_browser/app.html.ts
- M chrome/browser/resources/webui_browser/app.ts
- M chrome/browser/ui/webui_browser/browser.mojom
- M chrome/browser/ui/webui_browser/webui_browser_ui.cc
- M chrome/browser/ui/webui_browser/webui_browser_ui.h
- M chrome/browser/ui/webui_browser/webui_browser_window_mac.mm
Code-Review: +1 by Ken Buchanan, +1 by Keren Zhu
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |