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

Already on GitHub? Sign in to your account

width:auto for full-size images not working properly on Chrome #120

Closed
mpeshev opened this Issue Dec 8, 2012 · 13 comments

Comments

Projects
None yet
6 participants

mpeshev commented Dec 8, 2012

With regards to this PoC commit - mpeshev/_s@27c2064

The author has mentioned that the width: auto has been set for the sake of IE8. However, it's not working properly on WebKit (Chrome specifically) since images with attributes width/height are expanded to the content width. Could be tested with the Theme Unit Test data, Images Test, page 3 (or just an image with width/height attributes).

Disclosure: not tested on IE here. running on Linux.

Contributor

sixhours commented Feb 27, 2013

Confirmed, I've seen this happen as well.

I admit, I usually remove the line entirely because I'm not concerned with IE 8 support. In the past, we've added a special ID to the HTML tag to target IE browsers and only apply the fix to that browser, but I think in the interests of moving forward (IE 8 is now two versions past), we should remove it.

Collaborator

philiparthurmoore commented Feb 27, 2013

This seems really, really weird:

.site-header img,
.entry-content img,
.comment-content img,
.widget img {
    max-width: 100%; /* Fluid images for posts, comments, and widgets */
}
.site-header img,
.entry-content img,
.comment-content img[height],
img[class*="align"],
img[class*="wp-image-"] {
    height: auto; /* Make sure images with WordPress-added height and width attributes are scaled correctly */
}
.site-header img,
.entry-content img,
img.size-full {
    max-width: 100%;
    width: auto; /* Prevent stretching of full-size images with height and width attributes in IE8 */
}

In my projects I've only used this:

img {
    height: auto; /* Make sure images with WordPress-added height and width attributes are scaled correctly */
    max-width: 100%; /* Prevent images from overflowing their boundaries */
}
#ie8 img {
    width: auto; /* Prevent stretching of full-size images with height and width attributes in IE8 */
}

And even then the IE8 rules aren't needed anymore because IE8 support is going away.

Is this too broad brush?

img {
    height: auto; /* Make sure images with WordPress-added height and width attributes are scaled correctly */
    max-width: 100%; /* Prevent images from overflowing their boundaries */
}

In the themes I use it on I've not heard of any problems at all...

Contributor

sixhours commented Feb 27, 2013

@philiparthurmoore If I recall, setting height: auto on all images caused stretched images in tiled galleries. Not sure if that was the only reason we avoided it being applied too liberally, but it was a good one at the time. That may only apply on WP.com, though.

Could condense that block to:

.site-header img,
.entry-content img,
.comment-content img,
.widget img,
img.size-full {
    max-width: 100%; /* Fluid images for posts, comments, and widgets */
}
.site-header img,
.entry-content img,
.comment-content img[height],
img[class*="align"],
img[class*="wp-image-"] {
    height: auto; /* Make sure images with WordPress-added height and width attributes are scaled correctly */
}
Collaborator

philiparthurmoore commented Feb 27, 2013

If I recall, setting height: auto on all images caused stretched images in tiled galleries. Not sure if that was the only reason we avoided it being applied too liberally, but it was a good one at the time.

Confession: I've used only img with Able since it launched a while back and never had problems. And Tiled Galleries look great: http://abledemo.wordpress.com/tiled-galleries/.

I haven't seen a single real reason for not using only img for those rules yet.

Member

obenland commented Mar 11, 2013

@philiparthurmoore When you say "my projects", in which themes other than Able did you use the short form?

Collaborator

philiparthurmoore commented Mar 11, 2013

When you say "my projects", in which themes other than Able did you use the short form?

Most of them, launched or not, why?

Member

obenland commented Mar 11, 2013

Because I'd really like to try out your way with img {} and #ie8 img {}. It's so beautifully simple.

[EDIT]: Although I'd be in favor of using a class here: .ie8 img {} ;)

Collaborator

philiparthurmoore commented Mar 11, 2013

I thought we were about to fight, fight, fight! Ha! Like I said, I haven't run into a single problem but it's probably worth doing a test with the Visual Editor and image insertion again. I can certainly run some tests at some point this week for you.

Collaborator

philiparthurmoore commented Mar 11, 2013

For context, here's the entire style block; I tried to get too cute with aggressive pixel measurements which I'd like to eventually refactor if I ever find the time:

/* =Images
-------------------------------------------------------------- */

img {
    height: auto; /* Make sure images with WordPress-added height and width attributes are scaled correctly */
    max-width: 100%; /* Prevent images from overflowing their boundaries */
}
#ie8 img {
    width: auto; /* Prevent stretching of full-size images with height and width attributes in IE8 */
}
.aligncenter,
.alignleft,
.alignright,
.alignnone {
    display: block;
}
.aligncenter { /* Center-aligned images with no captions */
    margin: 26px auto;
}
.alignleft { /* Left-aligned images with no captions */
    float: left;
    margin: 26px 26px 26px 0;
}
.alignright { /* Right-aligned images with no captions */
    float: right;
    margin: 26px 0 26px 26px;
}
.alignnone { /* Images with no alignments and no captions */
    display: inline;
    margin: 0 0 -7px 0;
}
div.alignnone { /* Captions with no alignment */
    display: block;
}
.wp-caption { /* WordPress Captions */
    border: 1px solid rgb( 230, 230, 230 ); /* #e6e6e6 */
    border: 1px solid rgba( 230, 230, 230, 0.8 );
    max-width: 96%; /* Prevent captions from overflowing into sidebars and other adjacent content */
    margin-bottom: 26px; /* Value should match line-height */
    padding: 13px 8px 3px; /* WordPress automatically adds 5 pixels of padding on the right and left sides of captions, add balance */
}
.wp-caption img {
    display: block;
    margin: 0 auto 3px auto;
}
.wp-caption-text { /* This text is contained within the caption */
    font-family: 'Droid Serif', Georgia, Cambria, 'Times New Roman', Times, serif;
    font-size: 14px;
    font-size: 1.4rem;
    font-style: italic;
    margin: 0;
    padding: 2px 0 3px 0;
    text-align: center;
}

Pretty sure there's room for improvement here.

Contributor

mfields commented Mar 12, 2013

.wp-caption img {
    display: block;
}

This code would be a regression to #116

Collaborator

philiparthurmoore commented Mar 12, 2013

That's not being proposed. The above paste is for context.

Member

obenland commented Mar 12, 2013

@lancewillett and I discussed the approach shortly today during Twenty Thirteen office hours.

@ianstewart, @lancewillett, @mtias, would you mind sharing your experience with that during Twenty Eleven development?

Contributor

ianstewart commented Mar 26, 2013

I think we can drop any code that's in there for IE8.

iirc, The CSS is this granular to make sure images with and without height-width attributes scale correctly in Firefox, Webkit, and IE9+. If that's working in current versions of all the major browsers without height: auto I think we could simplify the CSS.

@obenland obenland closed this in f424711 Mar 27, 2013

@carl-alberto carl-alberto pushed a commit to carl-alberto/business-portfolio that referenced this issue May 27, 2016

@alkymst alkymst refs #120 - possible fix for 500 server error 88eb376
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment