MD Settings: closure breakages |
||
Issue description(ERROR) Error in: site_details_permission.js ## work2/src/chrome/browser/resources/settings/site_settings/site_details_permission.js:94: ERROR - Property permissionValue never defined on event.detail.item.dataset ## var action = event.detail.item.dataset.permissionValue; ## ^ ## ## work2/src/chrome/browser/resources/settings/site_settings/site_details_permission.js:104: ERROR - actual parameter 3 of SiteDetailsPermissionElement.prototype.setCategoryPermissionForOrigin does not match formal parameter ## found : (settings.PermissionValues<number>|string) ## required: number ## this.site.origin, '', value, this.category);
,
Apr 13 2016
,
Apr 13 2016
oh, I guess https://codereview.chromium.org/1880063002/ was an earlier CL. I'm really confused.
,
Apr 13 2016
The timeline, as I recall it, is as follows: Closure compiler starts producing a warning about cyclical GYP dependencies and skips all checks. Unfortunately, I don't notice the warning and submit a CL containing a couple of closure errors. I then submit another CL with a couple of different closure errors but around the same time TBR a CL with a fix for the *first* set of closure errors. To innocent bystanders it looks like no Closure error was fixed with my TBR'ed CL because the number of Closure errors remain 2. But the errors are different. I show up this morning and see two Closure errors remain and submit a CL for review to fix the remaining ones. That CL is referred to in comment 1 (in review).
,
Apr 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6d59b7cd346f67ec764d4da72ca4983a4d2d537 commit f6d59b7cd346f67ec764d4da72ca4983a4d2d537 Author: finnur <finnur@chromium.org> Date: Wed Apr 13 22:30:57 2016 Site Settings: Use only string values for permissions on the JS side. This also fixes a couple of Closure errors and improves the handling for pattern combinations containing embedded origins. BUG= 602899 , 543635 Review URL: https://codereview.chromium.org/1882793002 Cr-Commit-Position: refs/heads/master@{#387127} [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/constants.js [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/site_details_permission.html [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/site_details_permission.js [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/site_list.js [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/site_settings_behavior.js [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/ui/webui/settings/site_settings_handler.cc [modify] https://crrev.com/f6d59b7cd346f67ec764d4da72ca4983a4d2d537/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
,
Apr 13 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by finnur@chromium.org
, Apr 13 2016