Links widget does not generate width or height attributes when showing an image

[mbijon]

It's a best practice to include width and height attributes in image tags to prevent reflows when the browser renders the page. Both PageSpeed and YSlow include rules that measure this.

_walk_bookmarks() in /wp-includes/bookmark-template.php does support adding images to the output of the Links widget. However, it doesn't output the width or height attributes.

It is possible to size images with CSS instead of by tag attributes (and PageSpeed rules consider this acceptable). But it doesn't seem ideal to force users to set dimensions in CSS, when the Link edit page doesn't accept CSS directly. Plus, I think it's best to have all the meta info about something in one place, not to set an image in a WP link, and then the dimensions in a theme's CSS file.

[mbijon]

Uses PHP's getimagesize() to detect image size and output the dimensions

[mbijon]

No more PHP Notice when getimagesize() returns false

[mbijon]

I don't like that my method relies on getimagesize() and URL-based images that Links has. getimagesize() is used elsewhere in WP, but usually with a path to the image instead of the URL.

Would it be better to enhance the edit page for Links, instead of my method in _walk_bookmarks() ? This URL method increases roundtrips when the images are hosted remotely.

[ericmann]

I like the idea of it, actually. We really should be applying height/width attributes wherever we can, and this will definitely help with some content flow issues when building responsive layouts.

[ericmann]

Cleaning up the code slightly to allow for situations where the function getimagesize() doesn't work (i.e. GD Image Library isn't installed with PHP).

[mbijon]

Thanks Eric. Per your Twitter reference to /wp-includes/media.php, that's what I started from. My main change is using the 4th value returned by getimagesize() instead of building a string.

I'm not sure about throwing an error if GD Lib isn't installed (in attachment:ticket:19695:19695.3.diff​). I don't like doing that because GD isn't a requirement for installing WP, ​http://wordpress.org/about/requirements/. My method failed past that on purpose.

Note that PHP downgraded getimagesize() read errors to E_NOTICE in 5.2. I think that's the right approach, and that delivering content instead of an error msg is more important than having correctly-set image attributes.

[ericmann]

GD isn't a requirement, but this is the same way we handle it elsewhere. And, coming from having accidentally installed PHP at least once without GD, I can tell you that having notices like this in there is a lifesaver. It took me about 2 weeks to figure out why thumbnails weren't being auto-generated.

I mirrored my patch after the other patterns we follow when using getimagesize() in core for consistency. If one of the core leads wants to step in with an opinion, I'd be fine going either way, really.

[dd32]

I don't like this one bit, for the simple reason that getimagesize() is called on every request, quite potentially on a remote file, and due to the output not being scaled, it really provides little benefit for the performance hit it takes.

I'd prefer to see it support the width/height attributes if the item was added from the media library, otherwise just skip them. Of course, the Links stuff doesn't support the media library though, sounds like a great enhancement there for the day that links is moved from core to a plugin..

[mbijon]

Good point about this not being performant. What do you think about doing getimagesize() once on-save and then storing the values?

Adding media library now definitely isn't good. I just don't like leaving this unfixed up until links are finally moved to a plugin.

[chriscct7]

Links are no longer shown in WordPress core since #21307, this ticket is now invalid.