New issue
Advanced search Search tips

Issue 773233 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Pantheon detected as Unity

Reported by krystian...@gmail.com, Oct 10 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3230.0 Safari/537.36

Steps to reproduce the problem:
1. Run the browser on Pantheon environment (i.e. elementary OS) or set in your environment XDG_CURRENT_DESKTOP=Pantheon
2. Maximize the window

What is the expected behavior?

What went wrong?
Recent change to Chrome uses different method for detecting Unity desktop environment and hiding UI elements for maximized windows. On Unity there is systemwide top bar which displays those elements but Pantheon is missing them.

The bug is introduced with commint 0960ecfad7787c492f407054c9a9ba1b7134ef5d but the core reason for it is in base/nix/xdg_util.cc lines 61-72.

The condition from lines 61-63 matches "Unity" or "Pantheon" and declares DESKTOP_ENVIRONMENT_UNITY.

Please either remove the pantheon part or add proper Pantheon detection.

Did this work before? Yes 

Chrome version: 63.0.3230.0  Channel: dev
OS Version: elementary OS Loki
Flash Version:
 
Zrzut ekranu z 2017-10-10 12.11.18.png
67.8 KB View Download

Comment 1 by ajha@chromium.org, Oct 12 2017

Cc: ajha@chromium.org
Labels: -Pri-2 M-63 Pri-1
Owner: thomasanderson@chromium.org
Status: Assigned (was: Unconfirmed)
Don't have elementary OS set up to test and confirm this, based on C#0 assigning to thomasanderson@ for 0960ecfad7787c492f407054c9a9ba1b7134ef5d .

Tom@: Could you please take a look at this and update.

Thank you! 
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac4d6f43ab804283d2420dd6c92862b325981d1e

commit ac4d6f43ab804283d2420dd6c92862b325981d1e
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Fri Oct 13 20:14:20 2017

Add a new enum value to base::DesktopEnvironment for Pantheon

CL [1] fixed a bug where Chrome wouldn't add any frame buttons on a
maximized browser window.  The Unity desktop environment draws these
buttons for us in the top menu bar for maximized windows, so we only
disabled these buttons on the Unity DE.

However, we were also pretending that the Pantheon DE (used on
ElementaryOS) was Unity, so that libappindicator would work.  This CL
adds Pantheon as a new base::DesktopEnvironment to solve both of these
issues.

[1] https://crrev.com/0960ecfad7787c492f407054c9a9ba1b7134ef5d

BUG= 773233 
R=sky@chromium.org
CC=erg@chromium.org

Change-Id: If66f748e69c202d09c9ed6dfc0490bb2ce0d5609
Reviewed-on: https://chromium-review.googlesource.com/717479
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508792}
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/base/nix/xdg_util.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/base/nix/xdg_util.h
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/base/nix/xdg_util_unittest.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/browser/printing/printer_manager_dialog_linux.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/browser/ui/libgtkui/app_indicator_icon.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/browser/ui/libgtkui/gtk_ui.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/browser/ui/webui/settings_utils_linux.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/installer/linux/debian/postinst
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/installer/linux/debian/prerm
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/chrome/installer/linux/rpm/chrome.spec.template
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/components/os_crypt/key_storage_util_linux.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/net/proxy/proxy_config_service_linux.cc
[modify] https://crrev.com/ac4d6f43ab804283d2420dd6c92862b325981d1e/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc

Labels: Merge-Request-62
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 13 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 3 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-62 Merge-Rejected-62
What are the downsides if we punt this to M63? We are going to stable next week, and this change probably will not have any bake time in Dev or Beta. My recommendation is to wait until M63. 
Labels: Merge-Request-63
Status: Started (was: Assigned)
downside: close,minimize,maximize buttons will not be visible in ElementaryOS on a maximized Chrome window.

It was broken before, though, so I'd be fine waiting until M63.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 14 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by gov...@chromium.org, Oct 16 2017

** Bulk Edit **

Please merge your change to M63 branch 3239 before 5:00 PM PT Monday (10/16) so we can take it in for next dev release. Thank you.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 16 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff

commit f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Mon Oct 16 18:11:05 2017

[Merge to M63] Add a new enum value to base::DesktopEnvironment for Pantheon

> CL [1] fixed a bug where Chrome wouldn't add any frame buttons on a
> maximized browser window.  The Unity desktop environment draws these
> buttons for us in the top menu bar for maximized windows, so we only
> disabled these buttons on the Unity DE.
>
> However, we were also pretending that the Pantheon DE (used on
> ElementaryOS) was Unity, so that libappindicator would work.  This CL
> adds Pantheon as a new base::DesktopEnvironment to solve both of these
> issues.
>
> [1] https://crrev.com/0960ecfad7787c492f407054c9a9ba1b7134ef5d
>
> BUG= 773233 
> R=sky@chromium.org
> CC=erg@chromium.org
>
> Change-Id: If66f748e69c202d09c9ed6dfc0490bb2ce0d5609
> Reviewed-on: https://chromium-review.googlesource.com/717479
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508792}

TBR=sky@chromium.org
BUG= 773233 
NOTRY=true
NOTREECHECKS=true
NOPRESUBMIT=true

Change-Id: I176e570a04d76970830d6ebbf9b1c2cb3f4d2888
Reviewed-on: https://chromium-review.googlesource.com/721694
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#14}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/base/nix/xdg_util.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/base/nix/xdg_util.h
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/base/nix/xdg_util_unittest.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/browser/printing/printer_manager_dialog_linux.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/browser/ui/libgtkui/app_indicator_icon.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/browser/ui/libgtkui/gtk_ui.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/browser/ui/webui/settings_utils_linux.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/installer/linux/debian/postinst
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/installer/linux/debian/prerm
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/chrome/installer/linux/rpm/chrome.spec.template
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/components/os_crypt/key_storage_util_linux.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/net/proxy/proxy_config_service_linux.cc
[modify] https://crrev.com/f43e8bd92ca86cfbfdee1cb5dc453e6553be7aff/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc

Status: Fixed (was: Started)

Sign in to add a comment