[MD Extensions] "Not from Chrome web store" not shown. |
|||||||
Issue descriptionA 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.
,
Nov 28 2017
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.
,
Nov 28 2017
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?
,
Nov 28 2017
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.
,
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).
,
Nov 29 2017
,
Nov 29 2017
,
Nov 29 2017
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.
,
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").
,
Nov 29 2017
> The one concrete bound I've verified Eh, I meant "The one concrete bug I've verified"
,
Nov 29 2017
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
,
Nov 29 2017
FYI, sent a proposed fix at https://chromium-review.googlesource.com/c/chromium/src/+/795353.
,
Nov 29 2017
> 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).
,
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
,
Nov 29 2017
,
Dec 16 2017
This made it into the m64 branch. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dpa...@chromium.org
, Nov 28 2017