search

MicrosoftEdge / MSEdgeExplainers

Home for explainer documents originated by the Microsoft Edge team
Creative Commons Attribution 4.0 International
1.28k stars 199 forks source link

Feedback on TitleBarCustomization #165

Closed mgiuca closed 4 years ago

mgiuca commented 4 years ago

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.

  1. 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".

  2. 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 no theme_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.

  3. 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 whether window.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.

  4. 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...)

  5. 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.

  6. 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 called env() which allows CSS rules to access user-agent-supplied variables. Could we expose the overlay bounds as an env() 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.)

  7. 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!

amandabaker commented 4 years ago

Thanks Matt for the thorough feedback!

  1. 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".

Good point, we can tease apart the Chromium-specific bits from the any web standard proposals.

  1. 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 no theme_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.

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 theme_color for the background, and rather only suggest that theme_color is used when the title bar uses a flat color for the background. Then if the title bar has a gradient (e.g. Ubuntu), it can keep its normal styling and add use smoothed corners if necessary to make it look less awkward. (this would require more discussion as to where exactly the edges of the overlay are, but we can deal with that later if this option is considered). Thoughts on that?

Otherwise, we could mandate that the background of the caption controls overlay must use the theme_color, but I think we will have a hard time selling that.

  1. 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 whether window.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 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

  1. 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...)

The plan was to only support caption_controls_only when used in combination with the standalone display mode, and I think we should stick with that to keep the scope minimal and to not over complicate the scenario. Following that assumption, then it's safe to provide just one bounding rect. One note about not supporting minimal-ui: a developer could still add 'back' and 'refresh' buttons using javascript, so there is no loss of capability vs minimal-ui; it would just require additional coding.

  1. 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.

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 resize whenever the size of the caption controls overlay changes.

  1. 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 called env() which allows CSS rules to access user-agent-supplied variables. Could we expose the overlay bounds as an env() 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 agree that the JS APIs are not ideal for laying out the title bar. We considered adding a new CSS units (th and tw similar to vh and vw) or using CSS environment variables following the solution to the notch problem; however, we had an internal discussion around avoiding adding new properties/env variables that are scoped to such a small chunk of the web. These properties would have to be standardized, but would only be applicable to non-mobile PWAs that run in standalone mode and also enable the caption_controls_only member. These properties wouldn't apply to Android/iOS (those OSes have no need for caption controls and could instead use fullscreen mode to maximize screen space).

We can open this for discussion again and see if there are other solutions or if we should reconsider the env() option.

  1. 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.

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 standalone app if the developer sets the title of the webpage as 'bankofamerica.com' on their app hosted on 'evilsite.com'. It's not two origins, but it's a similar issue.

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.

g-ortuno commented 4 years ago

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.

amandabaker commented 4 years ago

@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

bpasero commented 4 years ago

Some feedback from VSCode point of view for this support in PWA:

Having a new window option caption_controls_only resonates with me because that is pretty much the model we have on macOS where we draw the title entirely using HTML and simply have the native window controls be put where they typically are because you cannot emulate them easily.

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 caption_controls_only - we would be able to provide a nice experience with a custom title too.

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.

amandabaker commented 4 years ago

@bpasero We've taken all platforms into consideration and plan to implement this everywhere except iOS and Android since they support "display": "fullscreen"). We'll just have to work through some design decisions on Linux since each distro looks a bit different, and we need to make sure the overlay looks good across all (or at least most) of them

alancutter commented 4 years ago

We considered adding a new CSS units (th and tw similar to vh and vw)

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.

amandabaker commented 4 years ago

@alancutter We've proposed an env() solution here: [css-env] Add environment variable for left/right bounds of notch. It would work in conjunction with the existing `env(safe-area-inset-*), and would apply to both the notch and to the caption controls overlay scenarios.

amandabaker commented 4 years ago

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.