-
Notifications
You must be signed in to change notification settings - Fork 213
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
Feedback on TitleBarCustomization #165
Comments
Thanks Matt for the thorough feedback!
Good point, we can tease apart the Chromium-specific bits from the any web standard proposals.
Thanks for calling this out. Apparently we have been thinking about this primarily from a Mac and Windows 10 perspective. A couple thoughts: Maybe we choose not to declare that the caption controls overlay will use the Otherwise, we could mandate that the background of the caption controls overlay must use the
I was trying to keep the example minimal so that's why I chose to omit feature detection, but I can add it for the sake of completeness
The plan was to only support
This one seems worthy of its own issue, but we can start the conversation here. I wasn't aware of extension buttons transiently populating the title bar, but now I see that this happens after clicking on an extension from within the "Settings and more" pane. Is there any situation in which there could be more than one extension icon present in the title bar at a time? What I just saw was that the icon is hidden after clicking anywhere outside of the extension popup, so I could only ever get one icon in the title bar at a time. If there can be at most 1 icon in the title bar at any time, then we could place the icon in the same location as the draggable region to prevent having to resize the caption controls overlay. If there could be multiple, then we will have to take the resizable route. With respect to the other "omnibox controls", are these all popup dialogs? If so, they shouldn't affect the width of the caption controls overlay or what the client draws. If not, can you give an example? (I see that you said password manager, but I thought that was a dialog) Ideally, I think the caption controls region should not have to resize, but if they do then it seems reasonable to fire
I agree that the JS APIs are not ideal for laying out the title bar. We considered adding a new CSS units ( We can open this for discussion again and see if there are other solutions or if we should reconsider the
Good point, we can add a security section to the explainer. If we go with the resizable caption controls overlay from (6), then we can easily accommodate the origin text that appears on startup. I also agree that we should ensure a few pixels of padding around the origin, both for the RTL security issue you mentioned and also for consistent styling. WRT to this: "For example, a phishing site could show 'bankofamerica.com' in the title bar in the same font that Chrome/Edge uses, and it would appear alongside 'evilsite.com'". This can already kind of happen in a Since the shortcut to launch the app would be the same text that the user saw during install of the app (in Chrome/Edge, the "name" from the manifest is used by default), there would be a mismatch in expected vs actual content upon launch. Operating under this assumption, the biggest threat that I can see is if someone installs this app on your computer and saves the name of this phishing app as "Bank of America" so that content matches expectations. However, I don't know how careful the average Joe is, so we might still have to do more to keep things safe. |
Re 5. You can see some of these omnibox controls by installing https://permission.site and clicking the location and microphone buttons. We also briefly show the app's origin when the app launches. |
#165 Changes include: - Add feature detection to example use #165 (item 3) - Add a Security Considerations section #165 (item 7) - Add detail on how the overlay will resize #165 (item 5) - `theme_color` is recommended for overlay bg color when supported by browser and OS #179 - Add section on where dialogs will be anchored
@g-ortuno Thanks for that link! It looks like a PWA could have quite a few icons in that top bar, so we will have resize the overlay both when the origin hides and when extension icons are added/removed from the overlay. I've added info about resizing to the doc under the section Overlaying the Caption Controls |
Some feedback from VSCode point of view for this support in PWA: Having a new window option On Windows we actually draw the window controls in HTML entirely but that is only because with the UI framework we are using there is no option to draw the controls natively. I would think that for PWA on Windows - given the new option I would like to add that for VSCode all platforms are equally important: Linux, Windows and macOS. I am sure a solution for custom title would be implemented for all target platforms and work properly. |
@bpasero We've taken all platforms into consideration and plan to implement this everywhere except iOS and Android since they support |
As someone who's worked on CSS animations I'd much rather see env() over new units. Animation code likes to represent length values as a linear sum of all the length units so we can perform simple vector arithmetic on them. Extending the number of dimensions of that vector adds overhead to the optimisation. |
@alancutter We've proposed an |
It looks like all the points here have been either addressed or broken out into their own issues, so I'm marking this as closed. |
Some general feedback on https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/TitleBarCustomization/explainer.md
Great proposal in general. Thanks Amanda and Jason for writing this up.
I think the document in general is a bit too specific to Chromium. Just in terms of marketing this as a web standard, it's tricky if you state problems as "in Chromium browsers, this looks like X, we need a new web standard to make it look like Y", a rebuttal of that may be "why not change the way Chromium browsers work? Why do we need a new web standard?" So I think it would be good to separate out the Chromium-specific parts from the web standards. You should still keep the screenshots and discuss how the UI will look in a real-world browser implementation (you can still mention Chromium). But I would not start out up front saying "we can't do this in Chromium", rather "there is no way for a developer to express this in the current Manifest standard".
I would like to see a discussion (probably a whole section) about the fact that this proposal will require the standard to mandate a lot more about how the title bar is rendered by the UA. Right now, the standard doesn't mandate anything about the appearance of the title bar. It doesn't have to use the theme colour, and if it does, it can do anything with it (e.g., make a gradient or a Win7-style glass effect). Your proposal implies that, at least in
caption_controls_only
mode, the UA will be required to display the overlay as an opaque rectangle of the app's theme colour. Without such a mandate, the site attempting to seamlessly match the in-client portion of the title bar with the non-client portion will have an ugly seam. So we do need to mandate that, and have some plan for if there is notheme_color
as well. In general, if the intention is that developers can seamlessly integrate their in-client UI with the UA-supplied UI, we need to specify enough of the UA's behaviour so that these sites will look right on any standards-compliant browser.Your examples should show how a site would "feature detect" this. Since some browsers won't (at least initially) support it, sites need a plan for what happens if the browser ignores the request to do
caption_controls_only
mode. In the current proposal, you could detect whetherwindow.menubar.controlOverlay
exists and then display your search box at the top of the window (so on those browsers, there would just be a conventional title bar and a search box underneath). I am thinking that maybe even a browser that supports this feature might turn it off on some platforms, e.g., Linux where there is a strong convention that window managers get to manage the caption area entirely and there is no API for clients to add stuff into that space. Note that for Chromium, we probably would support it on Linux because we make our own custom title bar for PWAs. But another browser might not support it on Linux.I don't think that it's enough for the UA to only be able to provide a single bounding rect. On many (if not all) platforms, we'll want to make two overlay rects, at least in minimal-ui mode: one for the left controls (such as back and reload) and one for the right controls (such as close, minimize and maximize). What is your plan to support the two sides of non-client controls? Would you just not allow this in minimal-ui mode? (I don't have a great solution for this other than supplying an array of overlay rects, which causes trouble later...)
I would like the overlay to be dynamically resizable by the UA. In particular, in Chrome, we display a bunch of transient content on the right side of the title bar, to the left of the 3-dot menu (site origin, extensions buttons, and other "omnibox controls" such as password manager). These can pop up and go away at any time. Therefore, we'd want to have the overlay be able to expand and shrink (at least horizontally) at the UA's discretion, and have the in-client UI dynamically adjust, Perhaps your intention is that the
resize
event would be fired whenever the overlay changes, which would cause the JS code computing the in-client UI to be re-run. Should state that explicitly.I don't like the fact that you need to run JS code to update your in-client UI to match the overlay. That is workable, but not ideal, especially since it means that UI isn't being updated by the browser's layout code, but by the much slower JS event loop. That means there's likely to be a user-noticeable lag between resizing the window (or the UA updating the overlay size) and the painting of the in-client UI. Ideally, all the code in your "
resizeTitleBar
" example could be done in CSS. I just learned (from an internal chat about this) that there's a CSS thing calledenv()
which allows CSS rules to access user-agent-supplied variables. Could we expose the overlay bounds as anenv()
variable, which would allow the majority of use cases for in-client title bar UI to be sized with CSS rather than JS? Note that this request may conflict with my above request to have multiple overlay rects, since that might make it much harder to expose as a CSS variable. (I don't know much about CSS; I just heard this was a thing, so apologies if this is unworkable for some other reason I haven't foreseen.)I think you should point out a security consideration that, by giving sites (partial) control over the title bar, there is a risk of spoofing. We designed the Chrome title bar UI very carefully with our security team, e.g., to make sure it is showing the security origin when the window first opens, so that sites cannot pretend to be some other site (since we don't have an address bar, this is a concern). We consider the title bar an "authoritative area". Obviously, this proposal still gives the UA a reserved space to display things like the origin, but we should be mindful that this also allows the site to potentially put spoofy things in the previously authoritative space. For example, a phishing site could show "bankofamerica.com" in the title bar in the same font that Chrome/Edge uses, and it would appear alongside "evilsite.com"; the user may not be aware of which of those two origins is "true". Consider what this looks like in a right-to-left language; is there a possibility to add ".google.com" to the right of the UA-supplied origin? Should we recommend that UAs have a minimum amount of space to the left/right of origin text that is part of the overlay, to avoid this attack? I haven't done a full security analysis, but these are a few things I would consider in a security review of this feature.
Thanks for reading my wall of text. You don't necessarily need to have answers to all of these right away, but it would be good to see them added to "open questions" section of the explainer. Looking forward to seeing this progress!
The text was updated successfully, but these errors were encountered: