New issue
Advanced search Search tips

Issue 603848 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Manifest: remove support for density

Project Member Reported by mlamouri@chromium.org, Apr 15 2016

Issue description

The Manifest specification has changed to no longer have density in it, mostly because developers did not understand it and were grossly misusing it leading to intended behaviour that looked like bugs. As a result, the spec editor of the specification (inc. me) decided to remove the property. See the discussion here: https://github.com/w3c/manifest/issues/450

We should remove the support from Chromium
 
Cc: dominickn@chromium.org
Dominick, is this something you would be interested to work on? Maybe a good start to get your hands a bit more on the Manifest code? :)
Cc: -dominickn@chromium.org
Owner: dominickn@chromium.org
Will look into this next week. Seems like it'll be a reasonably easy change to make. :)
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 21 2016

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

commit eaaf62daff59322dc31bebca2933c166c3f92e88
Author: dominickn <dominickn@chromium.org>
Date: Thu Apr 21 23:20:07 2016

Remove support for the web manifest icon density member.

The web manifest specification has been changed to remove the density
member, as this item was poorly understood and deployed
(discussion at https://github.com/w3c/manifest/issues/450).

This CL removes support for the density member from Chromium.
Substantive changes include:
 * the manifest parser ignores any density members
 * the manifest icon selector now purely selects icons based on their size
   in pixels, with the ideal and minimum sizes computed using the
   device's scale factor.

The manifest icon selector tests are improved and clarified in this CL
now that they do not need to use density.

BUG= 603848 

Review URL: https://codereview.chromium.org/1901363003

Cr-Commit-Position: refs/heads/master@{#388946}

[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/chrome/browser/manifest/manifest_icon_selector.cc
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/chrome/browser/manifest/manifest_icon_selector.h
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/chrome/browser/manifest/manifest_icon_selector_unittest.cc
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/common/manifest_manager_messages.h
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/public/common/manifest.cc
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/public/common/manifest.h
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/renderer/manifest/manifest_parser.cc
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/renderer/manifest/manifest_parser.h
[modify] https://crrev.com/eaaf62daff59322dc31bebca2933c166c3f92e88/content/renderer/manifest/manifest_parser_unittest.cc

Status: Fixed (was: Started)
Components: -Manifest Blink>AppManifest
Deprecate (and bulk edit/ move) Manifest

Sign in to add a comment