Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension backport #49

Merged
merged 15 commits into from
Mar 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions auditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,18 @@
*/
'use strict';

module.exports = function(audits) {
return function audit(results) {
var flattenedAudits = results.reduce(function(prev, curr) {
class Auditor {

_flattenArtifacts(artifacts) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be static

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha, I didn't mean to state that so strongly. More like they can be static since they aren't keeping track of state or anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine. I changed it.

return artifacts.reduce(function(prev, curr) {
return Object.assign(prev, curr);
}, {});
}

audit(artifacts, audits) {
const flattenedArtifacts = this._flattenArtifacts(artifacts);
return Promise.all(audits.map(audit => audit.audit(flattenedArtifacts)));
}
}

return Promise.all(audits.map(v => v(flattenedAudits)));
};
};
module.exports = Auditor;
47 changes: 47 additions & 0 deletions audits/manifest/background-color.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const manifestParser = require('../../helpers/manifest-parser');

class ManifestBackgroundColor {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains background_color';
}

static audit(inputs) {
let hasBackgroundColor = false;
const manifest = manifestParser(inputs.manifest).value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to reparse the manifest in every audit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can do it in the gather.


if (manifest) {
hasBackgroundColor = (!!manifest.background_color);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're going to actually want !!manifest.background_color.value here.

As we figure out our reporters, we're likely going to want to capture whether there's no background color because it wasn't included or because the background color was invalid (based on !!manifest.background_color.warning)

}

return {
value: hasBackgroundColor,
tags: ManifestBackgroundColor.tags,
description: ManifestBackgroundColor.description
};
}
}

module.exports = ManifestBackgroundColor;
29 changes: 20 additions & 9 deletions audits/service-worker/audit.js → audits/manifest/exists.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,26 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

module.exports = function(data) {
// Test the Service Worker registrations for validity.
let registrations = data.serviceWorkerRegistrations;
let activatedRegistrations = registrations.versions.filter(reg =>
reg.status === 'activated');
class ManifestExists {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Exists';
}

static audit(inputs) {
return {
value: inputs.manifest.length > 0,
tags: ManifestExists.tags,
description: ManifestExists.description
};
}
}

return {
'service-worker': activatedRegistrations.length > 0
};
};
module.exports = ManifestExists;
48 changes: 48 additions & 0 deletions audits/manifest/icons-192.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const manifestParser = require('../../helpers/manifest-parser');

class ManifestIcons192 {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains 192px icons';
}

static audit(inputs) {
let hasIcons = false;
const manifest = manifestParser(inputs.manifest).value;

if (manifest && manifest.icons) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kind of ridiculous, but you want something like:

if (manifest && manifest.icons.value) {
  const icons192 = manifest.icons.value.find(icon => icon.value.sizes.value.includes('192x192'));
  hasIcons = (!!icons192);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this one, seemed good :) I filter then bounce down to a bool. You get undefined if you can't find the entry in the array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, my suggestion is only necessary if they include multiple sizes on the same icon, so you have to parse the sizes entry. If they only have one size entry your check is equivalent to this

const icons192 = manifest.icons.raw.find(i => i.sizes === '192x192');
hasIcons = (!!icons192);
}

return {
value: hasIcons,
tags: ManifestIcons192.tags,
description: ManifestIcons192.description
};
}
}

module.exports = ManifestIcons192;
47 changes: 47 additions & 0 deletions audits/manifest/icons.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const manifestParser = require('../../helpers/manifest-parser');

class ManifestIcons {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains icons';
}

static audit(inputs) {
let hasIcons = false;
const manifest = manifestParser(inputs.manifest).value;

if (manifest) {
hasIcons = (!!manifest.icons);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manifest.icons.value.length > 0

}

return {
value: hasIcons,
tags: ManifestIcons.tags,
description: ManifestIcons.description
};
}
}

module.exports = ManifestIcons;
46 changes: 30 additions & 16 deletions audits/minify-html/audit.js → audits/manifest/name.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,35 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

module.exports = function(data) {
// See how compressed the HTML _could_ be if whitespace was removed.
// This could be a lot more aggressive.
let htmlNoWhiteSpaces = data.html
.replace(/\n/igm, '')
.replace(/\t/igm, '')
.replace(/\s+/igm, ' ');

let htmlLen = Math.max(1, data.html.length);
let htmlNoWhiteSpacesLen = htmlNoWhiteSpaces.length;
let ratio = Math.min(1, (htmlNoWhiteSpacesLen / htmlLen));

return {
'minify-html': ratio
};
};
const manifestParser = require('../../helpers/manifest-parser');

class ManifestName {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains name';
}

static audit(inputs) {
let hasName = false;
const manifest = manifestParser(inputs.manifest).value;

if (manifest) {
hasName = (!!manifest.name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!manifest.name.value

}

return {
value: hasName,
tags: ManifestName.tags,
description: ManifestName.description
};
}
}

module.exports = ManifestName;
47 changes: 47 additions & 0 deletions audits/manifest/short-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const manifestParser = require('../../helpers/manifest-parser');

class ManifestShortName {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains short_name';
}

static audit(inputs) {
let hasShortName = false;
const manifest = manifestParser(inputs.manifest).value;

if (manifest) {
hasShortName = (!!manifest.short_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!manifest.short_name.value

}

return {
value: hasShortName,
tags: ManifestShortName.tags,
description: ManifestShortName.description
};
}
}

module.exports = ManifestShortName;
47 changes: 47 additions & 0 deletions audits/manifest/start-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const manifestParser = require('../../helpers/manifest-parser');

class ManifestStartUrl {

static get tags() {
return ['Manifest'];
}

static get description() {
return 'Contains start_url';
}

static audit(inputs) {
let hasStartUrl = false;
const manifest = manifestParser(inputs.manifest).value;

if (manifest) {
hasStartUrl = (!!manifest.start_url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!manifest.start_url.value

}

return {
value: hasStartUrl,
tags: ManifestStartUrl.tags,
description: ManifestStartUrl.description
};
}
}

module.exports = ManifestStartUrl;