New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 789215 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 780164



Sign in to add a comment

[MD Extensions] "Not from Chrome web store" not shown.

Project Member Reported by dpa...@chromium.org, Nov 28 2017

Issue description

A user can install a local CRX file by dragging and dropping to the chrome://extensions page.

In the old UI, a text "Not from Chrome Web Store" was shown. In the new UI this is not shown anywhere, see screenshots.
 
not_from_cws.png
16.3 KB View Download
no_not_from_cws_message.png
10.4 KB View Download

Comment 1 by dpa...@chromium.org, Nov 28 2017

Labels: OS-Chrome OS-Linux

Comment 2 by dpa...@chromium.org, Nov 28 2017

Cc: rdevlin....@chromium.org
Looking at the code, there are a few more similar strings, that I don't think we are currently displaying in the MD version.

Not from Chrome Web Store.
Installed by a third party.
Installed because of dependent extension(s).

As well as
This extension contains malware.
This extension contains a serious security vulnerability.
This extension violates the Chrome Web Store policy.
Disabled by Chrome. This extension may be unsafe.

Comment 3 by jawag@chromium.org, Nov 28 2017

Cc: bettes@chromium.org
Nice catch -- these strings are pretty important, so I think this is a stable blocker.  Do we have the UI for where to show these strings, or do we still need to decide on that?

Comment 4 by dpa...@chromium.org, Nov 28 2017

Blocking: 780164
Still need to decide on the where to show these in the UI. The new UI assumes cards that have identical size, so I am not sure if those can fit within the cards.

We could put those somewhere in the details view, but then I don't know if that would be good enough from a security/privacy perspective.

Comment 5 by dpa...@chromium.org, Nov 28 2017

I actually see some code in MD Extensions that might be displaying the 2nd set of strings I posted at comment #2. Trying to verify (figuring out how to reproduce this cases is not trivial).
Owner: dpa...@chromium.org
Status: Assigned (was: Untriaged)

Comment 7 by dpa...@chromium.org, Nov 29 2017

Status: Started (was: Assigned)
Many of these are shown as errors/warnings in place of the extension description on the card.  See screenshot.

That's how we handle corrupted, not from the store, etc.

Installed by a third party is shown by the badging on the icon and the tooltip.

We should double-check if any of these are missing.
Selection_409.png
15.6 KB View Download

Comment 9 by dpa...@chromium.org, Nov 29 2017

Ok. I'll try to get an explicit list of all cases and compare before and after. 

The one concrete bound I've verified is the "Not from Chrome Web Store", see screenshot, where the new UI is erroneously displaying the opposite ("Source Chrome Web Store").

not_from_store_bug.png
146 KB View Download
> The one concrete bound I've verified

Eh, I meant "The one concrete bug I've verified"

Findings:

C++ sometimes populates a locationText field for each extension at [1]. The old UI uses that string when avaialeble at [2]. The new UI does not use that field at all. Moreover the utility method at [3] is missing a check for Location.UNKNOWN, which causes it to erroneously return SourceType.WEBSTORE for Location.UNKNOWN.

[1] https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/developer_private/extension_info_generator.cc?l=452-462
[2] https://cs.chromium.org/chromium/src/chrome/browser/resources/extensions/extension_list.js?l=622
[3] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_extensions/item_util.js?l=71-81
> As well as
> This extension contains malware.
> This extension contains a serious security vulnerability.
> This extension violates the Chrome Web Store policy.
> Disabled by Chrome. This extension may be unsafe.

I confirmed that the above strings are displayed as expected (attaching screenshot). So basically this bug requires no UX decision, it is just a matter of fixing the logic that populates the source icon and text (addressed per comment #12).
contains_malware.png
15.6 KB View Download
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 29 2017

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

commit b361969a4ccdd3780934b78adb73ded0e1461aa7
Author: dpapad <dpapad@chromium.org>
Date: Wed Nov 29 20:42:18 2017

MD Extensions: Stop displaying CWS as the source of unknown extensions.

Bug:  789215 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5b2ab2c062490c21d2b72e2518ff93f8f2f7a801
Reviewed-on: https://chromium-review.googlesource.com/795353
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520232}
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/browser/resources/md_extensions/detail_view.html
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/browser/resources/md_extensions/detail_view.js
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/browser/resources/md_extensions/item.js
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/browser/resources/md_extensions/item_util.js
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/test/data/webui/extensions/cr_extensions_browsertest.js
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/test/data/webui/extensions/extension_detail_view_test.js
[modify] https://crrev.com/b361969a4ccdd3780934b78adb73ded0e1461aa7/chrome/test/data/webui/extensions/extension_item_test.js

Status: Fixed (was: Started)
This made it into the m64 branch.

Sign in to add a comment