New issue
Advanced search Search tips

Issue 866170 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Extensions Click-to-Script: Don't grayscale active extensions

Project Member Reported by rdevlin....@chromium.org, Jul 20

Issue description

Extension icons in the toolbar should only be grayscaled if they can neither accept clicks (i.e., the extension action is disabled) *and* the extension cannot inject on the page.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 23

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

commit 516528f82e9ac80731371e1e117f01223034a1ba
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Jul 23 20:16:45 2018

[Extensions Click-to-Script] Adjust icon grayscaling behavior

Currently, extension icons in the toolbar are grayscaled when the
extension action (i.e., the page action or the browser action) is
disabled. However, this can be confusing, because it makes the extension
look disabled when it might be running on that page.

With runtime host permissions enabled, the extension icon should only be
grayscaled in the toolbar only if the action is currently disabled *and*
the extension cannot currently inject on the page. This addresses the
confusing "disabled" look, as well as preventing the strange case of
"clicking on the extension icon and it immediately becoming disabled."

Bug:  866170 

Change-Id: I00c239c6b4a35f52398e433d0a6ed357298e08c6
Reviewed-on: https://chromium-review.googlesource.com/1145630
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577235}
[modify] https://crrev.com/516528f82e9ac80731371e1e117f01223034a1ba/chrome/browser/extensions/extension_action_runner.h
[modify] https://crrev.com/516528f82e9ac80731371e1e117f01223034a1ba/chrome/browser/ui/extensions/extension_action_view_controller.cc
[modify] https://crrev.com/516528f82e9ac80731371e1e117f01223034a1ba/chrome/browser/ui/extensions/extension_action_view_controller.h
[modify] https://crrev.com/516528f82e9ac80731371e1e117f01223034a1ba/chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 23

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

commit 86600c69a09bdc9d38eabba200685e1d0606b456
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Mon Jul 23 23:49:36 2018

[jumbo] avoid ambiguities between extensions::features:: and features::

extensions/common/extension_features.h contains an extensions::features
namespace, which can be confused with chrome/common/chrome_features.h's
features namespace in jumbo builds.

Let's disambiguate these enough to get jumbo builds working again.

Followup to this CL, which uncovered the issue (but didn't cause it):
https://chromium-review.googlesource.com/c/chromium/src/+/1145630

Bug:  866170 
Change-Id: Iea5cca04c3c636014eae14f987c4aecd2dfb93bc
Reviewed-on: https://chromium-review.googlesource.com/1147521
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Cr-Commit-Position: refs/heads/master@{#577342}
[modify] https://crrev.com/86600c69a09bdc9d38eabba200685e1d0606b456/chrome/browser/ui/extensions/hosted_app_browser_controller.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30

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

commit eeb0ab47c29fda8b317939f7e9bb868d4dcbe9b2
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Aug 30 01:38:13 2018

[Extensions Click-to-Script] Fix grayscaling with content script access

With runtime host permissions, extension icons shouldn't be grayscaled
when they have access to the page. Correctly account for content script
permissions in this checking, and add unittests for the same.

Bug:  866170 

Change-Id: Icde8d0b121c2054cd812bb7342d4ce1fdf7fb00e
Reviewed-on: https://chromium-review.googlesource.com/1184024
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587390}
[modify] https://crrev.com/eeb0ab47c29fda8b317939f7e9bb868d4dcbe9b2/chrome/browser/ui/extensions/extension_action_view_controller.cc
[modify] https://crrev.com/eeb0ab47c29fda8b317939f7e9bb868d4dcbe9b2/chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment