Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2416733004:
Linux: Use FreeDesktop standard in .desktop entries (Closed)
|
Created:
4 years, 2 months ago by Tom (Use chromium acct) Modified:
4 years, 2 months ago Reviewers:
Lei Zhang CC:
chromium-reviews, grt+watch_chromium.org, pennymac+watch_chromium.org, Michael Moss, wfh+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionLinux: Use FreeDesktop standard in .desktop entries
BUG=655824, 502519
Committed: https://crrev.com/0297d67afdbb3980f7fd06daae3354fbd46cd618
Cr-Commit-Position: refs/heads/master@{#425237}
Patch Set 1 #Patch Set 2 : Revert install-sysroot thing #
Total comments: 4
Patch Set 3 : Remove X-Ayatana-Desktop-Shortcuts #
Total comments: 3
Patch Set 4 : Remove OnlyShowIn #Messages
Total messages: 15 (6 generated)
thomasanderson@google.com changed reviewers: + thestig@chromium.org
I really don't remember how these Ayatana bits came about. Also see https://bugs.chromium.org/p/chromium/issues/detail?id=502519 https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... File chrome/installer/linux/common/desktop.template (right): https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:114: X-Ayatana-Desktop-Shortcuts=NewWindow;NewIncognito Does this need to be updated? https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:116: [X-NewWindow Shortcut Group] Firefox has [Desktop Action NewWindow] and [Desktop Action NewPrivateWindow] on Trusty. See https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-...
Description was changed from ========== Linux: Add X- prefix in desktop.template BUG=655824 ========== to ========== Linux: Add X- prefix in desktop.template BUG=655824,502519 ==========
Description was changed from ========== Linux: Add X- prefix in desktop.template BUG=655824,502519 ========== to ========== Linux: Use FreeDesktop standard in .desktop entries BUG=655824,502519 ==========
https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... File chrome/installer/linux/common/desktop.template (right): https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:114: X-Ayatana-Desktop-Shortcuts=NewWindow;NewIncognito On 2016/10/13 23:12:35, Lei Zhang wrote: > Does this need to be updated? Done. https://codereview.chromium.org/2416733004/diff/20001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:116: [X-NewWindow Shortcut Group] On 2016/10/13 23:12:34, Lei Zhang wrote: > Firefox has [Desktop Action NewWindow] and [Desktop Action NewPrivateWindow] on > Trusty. See > https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-... This definitely feels more standard. Changed. https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... File chrome/installer/linux/common/desktop.template (right): https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:222: OnlyShowIn=Unity; Do you know why TargetEnvironment=Unity was originally added? Can we just remove this? Also, desktop-file-validate gives warnings for this: chrome/installer/linux/common/desktop.template: warning: key "OnlyShowIn" in group "Desktop Action NewWindow" is deprecated chrome/installer/linux/common/desktop.template: warning: key "OnlyShowIn" in group "Desktop Action NewPrivateWindow" is deprecated There were errors before when using TargetEnvironment=Unity
https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... File chrome/installer/linux/common/desktop.template (right): https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:222: OnlyShowIn=Unity; On 2016/10/14 00:00:01, Tom Anderson wrote: > Do you know why TargetEnvironment=Unity was originally added? Nope. As I said, the lost some history with the transition into an internal repo and then back out. > Can we just remove this? I'd say yes. See the discussion linked from https://bugs.freedesktop.org/show_bug.cgi?id=66712
https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... File chrome/installer/linux/common/desktop.template (right): https://codereview.chromium.org/2416733004/diff/40001/chrome/installer/linux/... chrome/installer/linux/common/desktop.template:222: OnlyShowIn=Unity; On 2016/10/14 01:23:44, Lei Zhang wrote: > On 2016/10/14 00:00:01, Tom Anderson wrote: > > Do you know why TargetEnvironment=Unity was originally added? > > Nope. As I said, the lost some history with the transition into an internal repo > and then back out. > > > Can we just remove this? > > I'd say yes. See the discussion linked from > https://bugs.freedesktop.org/show_bug.cgi?id=66712 Done.
lgtm
The CQ bit was checked by thomasanderson@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Linux: Use FreeDesktop standard in .desktop entries BUG=655824,502519 ========== to ========== Linux: Use FreeDesktop standard in .desktop entries BUG=655824,502519 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Linux: Use FreeDesktop standard in .desktop entries BUG=655824,502519 ========== to ========== Linux: Use FreeDesktop standard in .desktop entries BUG=655824,502519 Committed: https://crrev.com/0297d67afdbb3980f7fd06daae3354fbd46cd618 Cr-Commit-Position: refs/heads/master@{#425237} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/0297d67afdbb3980f7fd06daae3354fbd46cd618 Cr-Commit-Position: refs/heads/master@{#425237} |
