manifest not accepted when specifying chrome_settings_overrides w/o search_provider
Reported by
kboe...@googlemail.com,
Oct 4
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0 Steps to reproduce the problem: 0. Use an existing Extension 1. Add "chrome_settings_overrides" section to manifest.json 2. Provide attribute "homepage": "extensionwebsite.html" for this section 3. Try to load extension What is the expected behavior? manifest.json is accepted with having attribute "homepage" (and "startup_pages") set. What went wrong? When not having attribute "search_provider" set, an error is thrown: Empty dictionary for 'chrome_settings_overrides'. Could not load manifest. Did this work before? N/A Chrome version: Version 69.0.3497.100 (Official Build) (64-bit) Channel: stable OS Version: 10.0 Flash Version: According to documentation (https://developer.chrome.com/extensions/settings_override) all three attributes are optional.
,
Oct 5
kboenke@ Thanks for the issue. Request you to provide a sample extension where this issue can be reproduced, which will help in further triaging. Thanks..
,
Oct 5
Created and attached. If you add a properly formatted "search_provider" to "chrome_settings_overrides" it will be loaded.
,
Oct 5
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 8
,
Nov 16
Able to reproduce the issue on chrome reported version# 69.0.3497.100, latest stable# 70.0.3538.102 and on latest chrome# 72.0.3611.0 with file provided in comment# 3 using Windows-10, Mac 10.12.6 and Ubuntu 14.04. As this issue is seen from M-60(60.0.3112.0), hence considering this issue as Non-Regression and marking it as Untriaged. Thanks!
,
Nov 16
I think I found what's going on here. In parsing the settings overrides, we correctly try to parse homepages and startup pages, and if those fail, we set the error (e.g., here [1]). If there is an error, we'll return empty values for those overrides. However, we don't check if there is an error after these steps, and instead just look at whether all the values are empty [2], returning an "empty dictionary" warning if they are. We should change this so that we check the error after each Parse*() call, and early out if there was one found. Kelvin, can you take a quick look at this one, when you have bandwidth? [1] https://chromium.googlesource.com/chromium/src/+/16cdbd08d4bc9f96291d06447254a58e3c7f5f22/chrome/common/extensions/manifest_handlers/settings_overrides_handler.cc#46 [2] https://chromium.googlesource.com/chromium/src/+/16cdbd08d4bc9f96291d06447254a58e3c7f5f22/chrome/common/extensions/manifest_handlers/settings_overrides_handler.cc#142 |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by phanindra.mandapaka@chromium.org
, Oct 4