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

webui: Use client side decorations for browser #5057

Closed
wants to merge 2 commits into from

Conversation

halfline
Copy link
Contributor

At the moment we show a titlebar with "Mozilla Firefox" in the title.

It looks much better to use client side decorations where the titlebar is rendered different and without Firefox branding.

This commit achieves that by:

  1. Undoing the config override that forces the server side titlebar to be on
  2. Putting MOZ_GTK_TITLEBAR_DECORATION=client in the environment so firefox (which is currently running as root segregated from the session) knows its okay to use client side decorations.

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a screenshot before and after in the PR description?

@halfline
Copy link
Contributor Author

Screenshot from 2023-08-21 09-19-53
Screenshot from 2023-08-21 09-16-07

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@VladimirSlavik
Copy link
Contributor

/packit test

@VladimirSlavik
Copy link
Contributor

/packit build

@VladimirSlavik
Copy link
Contributor

/kickstart-test --testtype smoke

@AdamWill
Copy link
Contributor

Does this fix or improve the title of the window in the overview, if you open it? How about the icon?

@stransky
Copy link

halfline I wonder why do you need to set MOZ_GTK_TITLEBAR_DECORATION=client as it should be the default?

If Firefox is running with system titlebar enabled it means system environment auto-detection is broken or non-existent. Is that expected?

I mean it's okay to set MOZ_GTK_TITLEBAR_DECORATION=client but Firefox usually gets all config from system so if there's anything special you want to achieve (enabled system titlebar, different title etc.) there may be easier methods available than modify whole system setup for it.

@halfline
Copy link
Contributor Author

halfline commented Aug 29, 2023

halfline I wonder why do you need to set MOZ_GTK_TITLEBAR_DECORATION=client as it should be the default?

If Firefox is running with system titlebar enabled it means system environment auto-detection is broken or non-existent. > Is that expected?

I think it's because it's running as root segregated from the rest of the session. It may not be needed after #5058 but I haven't test that.

there's anything special you want to achieve (enabled system titlebar, different title etc.) there may be easier methods available than modify whole system setup for it.

Actually if you have insight into less fragile ways to allow list external url handlers than editing the sqlite database (see #5059 (comment) ), I'd be grateful.

@stransky
Copy link

I think it's because it's running as root segregated from the rest of the session. It may not be needed after #5058 but I haven't test that.

I don't think root is related here. Firefox uses XDG_* env variables to get system setup. I think if you set 'XDG_CURRENT_DESKTOP=GNOME' you'll get the same Firefox look as in regular gnome session (CSD enabled, titlebar off).

@stransky
Copy link

Actually if you have insight into less fragile ways to allow list external url handlers than editing the sqlite database (see #5059 (comment) ), I'd be grateful.

I'm sure this can be handled somehow. Will look at it.

@halfline
Copy link
Contributor Author

I don't think root is related here. Firefox uses XDG_* env variables to get system setup. I think if you set 'XDG_CURRENT_DESKTOP=GNOME' you'll get the same Firefox look as in regular gnome session (CSD enabled, titlebar off).

Well it's not related to the uid of the running process directly, but escalating to root cleaves the app from aspects of the session and the environment (including XDG_CURRENT_DESKTOP), so that does explain it. We could make sure XDG_CURRENT_DESKTOP=GNOME bubbles through. Does CSD get used on KDE as well? (just thinking about making sure things are futureproofed for the kde live images later on)

@stransky
Copy link

Well it's not related to the uid of the running process directly, but escalating to root cleaves the app from aspects of the session and the environment (including XDG_CURRENT_DESKTOP), so that does explain it. We could make sure XDG_CURRENT_DESKTOP=GNOME bubbles through. Does CSD get used on KDE as well? (just thinking about making sure things are futureproofed for the kde live images later on)

Yes, Firefox uses CSD on all systems where XDG_CURRENT_DESKTOP is set, which includes KDE,Gnome,Mate,XFCE,Unity and so on. Non-decorated windows are used on twm and similar ones where XDG_CURRENT_DESKTOP is missing.

If XDG_CURRENT_DESKTOP is set it also removes system titlebar - not sure if you want it.
And if system titlebar is removed you have full control which titlebar buttons are shown in Firefox (I think there was an issue that Firefox shows a titlebar with close button).

@halfline
Copy link
Contributor Author

alright, let's just let XDG_CURRENT_DESKTOP bubble through then.

@stransky
Copy link

alright, let's just let XDG_CURRENT_DESKTOP bubble through then.

Okay. Feel free to ping me if you have any Firefox related issues.

At the moment we show a titlebar with "Mozilla Firefox" in the
title.

It looks much better to use client side decorations where the
titlebar is rendered different and without Firefox branding.

This commit achieves that by:

1. Undoing the config override that forces the server side titlebar to be on
2. Making sure more of the environment is bubbled through anaconda to
   the webui launcher. In particular, we need XDG_CURRENT_DESKTOP, but
   this commit brings it all through, so firefox runs in an environment
   as close to getting run directly by the live user as possible.
3. Two exceptions are XAUTHORITY and XDG_RUNTIME_DIR which need to
   remain unset until we can run firefox as a normal user instead of root.
When the user clicks the close button, it currently displays:

----
🌐 127.0.0.1

This page is asking you to confirm that you want to
leave — information you’ve entered may not be saved.

                      [ Stay on Page ] [ Leave Page ]
----

That is obviously not a great user experience.

Anaconda already has a way to abort the install and reboot, so
the close button isn't really adding much.

This commit hides the close button.
@halfline
Copy link
Contributor Author

halfline commented Sep 5, 2023

okay i added an additional commit to hide the close button.

@halfline
Copy link
Contributor Author

halfline commented Sep 5, 2023

@VladimirSlavik time to land this?

@M4rtinK M4rtinK added the blocked Don't merge this pull request! label Sep 6, 2023
@M4rtinK
Copy link
Contributor

M4rtinK commented Sep 6, 2023

I have a solution with similar end result almost ready as part of #5112 which also handles how the application looks like in the Gnome Shell overview mode. Please don't merge this until after #5112 is merged, if still applicable, to avoid potential breakage. Thanks!

@halfline
Copy link
Contributor Author

let's close this, now that #5112 has landed this isn't offering much I guess

@halfline halfline closed this Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Don't merge this pull request! f39 webui
Development

Successfully merging this pull request may close these issues.

7 participants