Skip to content

Commit

Permalink
feat: tooltips for all formatted URLs (#2398)
Browse files Browse the repository at this point in the history
* feat: tooltips for all formatted URLs

* add testg

* remove comment

* feedback

* undo total byte weight change
  • Loading branch information
patrickhulce authored May 31, 2017
1 parent 84d3664 commit 6d35fa8
Show file tree
Hide file tree
Showing 12 changed files with 50 additions and 15 deletions.
2 changes: 1 addition & 1 deletion lighthouse-core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
* @return {?Object}
*/
static computeWaste(image, viewportDimensions) {
const url = URL.getURLDisplayName(image.src, {preserveQuery: true});
const url = URL.elideDataURI(image.src);
const totalPixels = image.clientWidth * image.clientHeight;
const visiblePixels = this.computeVisiblePixels(image.clientRect, viewportDimensions);
// Treat images with 0 area as if they're offscreen. See https://github.com/GoogleChrome/lighthouse/issues/1914
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
return;
}

const url = URL.getURLDisplayName(image.url);
const url = URL.elideDataURI(image.url);
const jpegSavings = UsesOptimizedImages.computeSavings(image, 'jpeg');

results.push({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit {
}

// remove duplicates
const url = URL.getURLDisplayName(record.url);
const url = URL.elideDataURI(record.url);
const isDuplicate = results.find(res => res.url === url &&
res.totalBytes === record.resourceSize);
if (isDuplicate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
* @return {?Object}
*/
static computeWaste(image, DPR) {
const url = URL.getURLDisplayName(image.src);
const url = URL.elideDataURI(image.src);
const actualPixels = image.naturalWidth * image.naturalHeight;
const usedPixels = image.clientWidth * image.clientHeight * Math.pow(DPR, 2);
const wastedRatio = 1 - (usedPixels / actualPixels);
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/byte-efficiency/uses-webp-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class UsesWebPImages extends ByteEfficiencyAudit {
return;
}

const url = URL.getURLDisplayName(image.url);
const url = URL.elideDataURI(image.url);
const webpSavings = OptimizedImages.computeSavings(image, 'webp');

results.push({
Expand Down
5 changes: 1 addition & 4 deletions lighthouse-core/audits/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

const Audit = require('./audit');
const Formatter = require('../report/formatter');
const URL = require('../lib/url-shim');

class Deprecations extends Audit {
/**
Expand Down Expand Up @@ -61,12 +60,10 @@ class Deprecations extends Audit {
});

const deprecationsV2 = entries.filter(log => log.entry.source === 'deprecation').map(log => {
// CSS deprecations can have missing URLs and lineNumbers. See https://crbug.com/680832.
const url = URL.isValid(log.entry.url) ? URL.getURLDisplayName(log.entry.url) : '';
return {
type: 'code',
text: log.entry.text,
url,
url: log.entry.url,
source: log.entry.source,
lineNumber: log.entry.lineNumber
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
'use strict';

const Audit = require('../audit');
const URL = require('../../lib/url-shim');
const Formatter = require('../../report/formatter');
const scoreForWastedMs = require('../byte-efficiency/byte-efficiency-audit').scoreForWastedMs;

Expand Down Expand Up @@ -77,7 +76,7 @@ class LinkBlockingFirstPaintAudit extends Audit {
endTime = Math.max(item.endTime, endTime);

return {
url: URL.getURLDisplayName(item.tag.url),
url: item.tag.url,
totalKb: `${Math.round(item.transferSize / 1024)} KB`,
totalMs: `${Math.round((item.endTime - startTime) * 1000)}ms`
};
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/audits/is-on-https.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class HTTPS extends Audit {
return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
const insecureRecords = networkRecords
.filter(record => !HTTPS.isSecureRecord(record))
.map(record => ({url: URL.getURLDisplayName(record.url, {preserveHost: true})}));
.map(record => ({url: URL.elideDataURI(record.url)}));

let displayValue = '';
if (insecureRecords.length > 1) {
Expand All @@ -77,7 +77,7 @@ class HTTPS extends Audit {
details: {
type: 'list',
header: {type: 'text', text: 'Insecure URLs:'},
items: insecureRecords.map(record => ({type: 'text', text: record.url})),
items: insecureRecords.map(record => ({type: 'url', text: record.url})),
}
};
});
Expand Down
14 changes: 14 additions & 0 deletions lighthouse-core/lib/url-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,20 @@ URL.getURLDisplayName = function getURLDisplayName(url, options) {
return Util.getURLDisplayName(new URL(url), options);
};

/**
* Limits data URIs to 100 characters, returns all other strings untouched.
* @param {string} url
* @return {string}
*/
URL.elideDataURI = function elideDataURI(url) {
try {
const parsed = new URL(url);
return parsed.protocol === 'data:' ? url.slice(0, 100) : url;
} catch (e) {
return url;
}
};

// There is fancy URL rewriting logic for the chrome://settings page that we need to work around.
// Why? Special handling was added by Chrome team to allow a pushState transition between chrome:// pages.
// As a result, the network URL (chrome://chrome/settings/) doesn't match the final document URL (chrome://settings/).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ function generateImage(type, originalSize, webpSize, jpegSize) {
return {
isBase64DataUri: isData,
url: isData ?
`data:image/${type};base64,reaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaly long` :
`data:image/${type};base64,reaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaly ` +
'reaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaly long' :
`http://google.com/image.${type}`,
mimeType: `image/${type}`,
originalSize, webpSize, jpegSize
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/is-on-https-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Security: HTTPS audit', () => {
])).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.displayValue.includes('request found'));
assert.deepEqual(result.extendedInfo.value[0], {url: 'insecure.com/image.jpeg'});
assert.deepEqual(result.extendedInfo.value[0], {url: 'http://insecure.com/image.jpeg'});
});
});

Expand Down
24 changes: 24 additions & 0 deletions lighthouse-core/test/lib/url-shim-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,30 @@ describe('URL Shim', () => {
});
});

describe('elideDataURI', () => {
it('elides long data URIs', () => {
let longDataURI = '';
for (let i = 0; i < 1000; i++) {
longDataURI += 'abcde';
}

const elided = URL.elideDataURI(`data:image/jpeg;base64,${longDataURI}`);
assert.ok(elided.length < longDataURI.length, 'did not shorten string');
});

it('returns all other inputs', () => {
const urls = [
'data:image/jpeg;base64,foobar',
'https://example.com/page?query=string#hash',
'http://example-2.com',
'chrome://settings',
'blob://something',
];

urls.forEach(url => assert.equal(URL.elideDataURI(url), url));
});
});

describe('equalWithExcludedFragments', () => {
it('correctly checks equality of URLs regardless of fragment', () => {
const equalPairs = [
Expand Down

0 comments on commit 6d35fa8

Please sign in to comment.