feat: tooltips for all formatted URLs#2398
Conversation
| @@ -62,11 +61,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. | |||
There was a problem hiding this comment.
Shall we remove the comment?
brendankenny
left a comment
brendankenny
left a comment
There was a problem hiding this comment.
is there a general rule for this? When should an audit author be sure to shorten data URLs? Should _renderURL take care of that instead?
| }; | ||
|
|
||
| /** | ||
| * @param {string} url |
| * @param {string} url | ||
| * @return {string} | ||
| */ | ||
| URL.elideDataURI = function elideDataURI(url) { |
There was a problem hiding this comment.
I guess preferring elideDataUri is already a losing battle :(
There was a problem hiding this comment.
ha, yeah I've mostly given in at this point
| URL.elideDataURI = function elideDataURI(url) { | ||
| try { | ||
| const parsed = new URL(url); | ||
| return parsed.protocol === 'data:' ? url.slice(0, 100) : url; |
There was a problem hiding this comment.
no clue when it could matter, but what about parsed.href.slice(0, 100)
There was a problem hiding this comment.
that seems slightly more confusing to me since I started second guessing if parsed.href is somehow different from url, is there a reason you prefer it?
There was a problem hiding this comment.
that seems slightly more confusing to me since I started second guessing if parsed.href is somehow different from url, is there a reason you prefer it?
making a reader second guess is a downside, but it does mean that any canonicalization or whatever that the URL constructor does is included.
OTOH, I have no clue if it would actually do anything to the beginning of a data URL that parsed successfully :)
parsed.toString().slice(0, 100) could be less confusing but does the same thing
There was a problem hiding this comment.
it does mean that any canonicalization or whatever that the URL constructor does is included
I'm a little confused why we want this, it's going to be reprocessed by a new URL by the renderer in _renderUrl anyhow, so why would we want to mess with it more here?
There was a problem hiding this comment.
yeah, I guess I'm thinking about URL.elideDataURI in isolation/other future uses. To me it makes sense to get a shortened version of the canonicalized URL out of calling it, but without a known change that could be made to it, up to you
when there's a reasonable chance that the URL can be massive i.e. it can be a very large image, here I've done it to all audits where we might expect an image and left it off for those we don't |
is it this way (vs just in |
correct-o |
| URL.elideDataURI = function elideDataURI(url) { | ||
| try { | ||
| const parsed = new URL(url); | ||
| return parsed.protocol === 'data:' ? url.slice(0, 100) : url; |
There was a problem hiding this comment.
yeah, I guess I'm thinking about URL.elideDataURI in isolation/other future uses. To me it makes sense to get a shortened version of the canonicalized URL out of calling it, but without a known change that could be made to it, up to you
3 participants
extends the goodness of #2312 to all audits formatting URLs
also adds an
elideDataURImethod to url shim to prevent enormous images from filling the LHR multiple times