manifest.json not parsed when encoded with BOM
Reported by
rblackma...@gmail.com,
Mar 30 2017
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Steps to reproduce the problem: 1. Add a valid manifest.json file to a website that is encoded in UTF-8 with BOM. 2. Verify that chromium does not parse the manifest. 3. Encode same file without BOM 4. Verify that chromium does parse the manifest. What is the expected behavior? Chromium should parse the manifest.json. What went wrong? Chromium does not parse the manifest.json. Did this work before? No Does this work in other browsers? Yes Chrome version: 57.0.2987.133 Channel: stable OS Version: 10.0 Flash Version: This prevents behavior such as Chrome offering to add the application to the home screen.
,
Apr 10 2017
Tested this issue on Windows 10 with chrome #57.0.2987.133 and observed the same output as manifest.png rblackman24@ could you please help us with the actual and expected output screenshots for further triage. Thank You...
,
Apr 10 2017
Thank you for providing more feedback. Adding requester "kkaluri@chromium.org" to the cc list and removing "Needs-Feedback" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 10 2017
kkaluri@chromium.org: Attached are an expected an actual screenshot showing the results of a very simple manifest.json file. Screenshots were taken on Windows 10 with Chrome 57.0.2987.133 (64-bit). Note: Re-uploading files because I had the screenshot names reversed.
,
Apr 21 2017
BOM is INVALID in JSON files (see https://tools.ietf.org/html/rfc7159#section-8.1 ) so the current behavior is correct. But the dev-tools should show the problem by name.
,
Apr 21 2017
"In the interests of interoperability, implementations that parse JSON texts MAY ignore the presence of a byte order mark rather than treating it as an error." I interpret this that Chromium could still choose to parse the file and be compliant with the spec.
,
Apr 21 2017
+dominickn to triage. It'd be valuable if there was an explicit check in manifest_parser to issue an error if a BOM is found in the manifest. Otherwise it'd hard to debug why the manifest isn't determined as parseable. Either that or we handle the BOM without a problem. :)
,
Apr 21 2017
,
Apr 21 2017
,
Apr 22 2017
c#9, c#10: This has nothing to do with extensions. This is the web app manifest, and the description clearly says that having a BOM works in other browsers, making it an interop issue. I'll investigate next week.
,
Apr 22 2017
#11 - right, sorry. I think a link from an extension-related issue threw me off here (as well as "manifest.json").
,
Apr 24 2017
c#12: no worries! This is actually an issue in devtools, which uses JSON.parse to parse the manifest (internally we use the C++ JSON parser for add to homescreen and app banners). See third_party/WebKit/Source/devtools/front_end/resources/AppManifestView.js:109. It looks like JSON.parse doesn't like JSON files with a BOM as the first byte: [16828:16828:0424/140101.701225:ERROR:CONSOLE(38)] "Uncaught (in promise) SyntaxError: Unexpected token in JSON at position 0", source: chrome-devtools://devtools/bundled/resources/resources_module.js (38) Over to pfeldman - perhaps a warning can be surfaced in devtools if the parsing throws a SyntaxError? There might be a more general interop question about what JSON.parse should do if other browsers are handling a BOM.
,
Apr 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab0ffaa4b4181f089c428c8f474e14eddc353b73 commit ab0ffaa4b4181f089c428c8f474e14eddc353b73 Author: pfeldman <pfeldman@chromium.org> Date: Tue Apr 25 05:25:24 2017 DevTools: trim the leading BOM char when parsing web manifest. BUG= 706926 Review-Url: https://codereview.chromium.org/2837943003 Cr-Commit-Position: refs/heads/master@{#466886} [modify] https://crrev.com/ab0ffaa4b4181f089c428c8f474e14eddc353b73/third_party/WebKit/Source/devtools/front_end/resources/AppManifestView.js
,
Apr 25 2017
,
May 5 2017
Deprecate (and bulk edit/ move) Manifest |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by ranjitkan@chromium.org
, Apr 4 2017