IE Image Link Fix

Agreed.

Yes, but what about captions:

<span class="inline left"><img /><span class="caption">Caption</span></span>

Wouldn't this patch mess up how images and captions are aligned? I didn't test it (sorry) but it seems wrong. How about changing the surrounding span to a div and floating that instead?

I think you are right, although I'm not 100% sure, too. However, DIVs are not an option because they span over all available width unless the width is explicitly defined via CSS.

We could try to turn this into

span.left {
  float: left;
}
span.left img {
  margin: 0 5px 5px 0;
}
span.right {
  float: right;
}
span.right img {
  margin: 0 0 5px 5px;
}

Eeerrm... where's my head?!

we're trying to apply 'float: left', and 'float: right' to spans, which are inline elements, but float only makes sense for block elements.

Seems like we need something completely different to solve this.

Why not use

<div style="width: $image-width">
  <img />
  <span>Caption</span>
</div>

Let's stick to DIV containers, including the caption.

Attached patch also renames those ambigious CSS alignment classes to inline-[align].

I've tested your patch with Garland and it seems that it solves the bug.

A problem I see with that inline classes. Why do we give class="inline" to a float-element. I think class="inline" should be given to none-aligned images. The css code for inline could be:

.inline {
  display: inline;
}

Styling of images can take over another class instead of inline.

I agree that 'inline' is somewhat misleading. However, it's valuable to have a common class name for all inline images (so all inline images can be styled equivalent easily) and AFAIK the class name is not ambigious (in Garland theme at least).

Since I'm also the maintainer of Inline module and there is already a draft to form an Inline API (a future release of Image Assist could be based on), I would not alter this class name now, since it would be renamed back to 'inline' when Inline API will be released and integrated someday.

Anyway, your point for styling of non-aligned images is not yet solved. We might implement

div.inline-none {
  display: inline;
}

However, what happens to captions for non-aligned images? How have those captions been styled in previous versions?

I don't suppose we could have solved this with:

span.inline-left img {
  display: block;
  float: left;
  margin: 5px 5px 5px 0px;
}

That is, stick with spans but add display: block. That way we wouldn't need the style attribute on the div. Don't know if it would work in IE though.

Anyway, I'm OK with the last patch as it is. I don't think the naming confusion is such a big deal. "Inline" refers to "inline images" in this case – and I don't think it's really desirable to map class names to the style it's using anyway. After all, you're supposed to name classes so that they make sense WRT the page structure: like "content", "search" etc., not "big", "wide" etc.

Reverting to SPANs, but always including alignment class and removing the width at all.

If an image is centered, the width for the caption is not limited. Additionally, the caption is centered, too. However, we could add text-align: left; to the caption. I'd like to hear your votes on that.

Removing the width from captions at all is not possible, unless we find a nifty CSS trick to limit the caption's width to the width of an left or right aligned image.

OT: The 'link to url' option is not working, $attributes['url'] isn't set. I'll file a separate issue for that.

So... style attributes are still necessary for captions. Hm. Maybe divs aren't so bad after all. If we used divs with the width set in a style attribute then maybe the captions wouldn't need to specify it. (That 2 pixel subtraction of caption widths could be done with padding instead.)

Spans or divs... I don't know, either way is fine with me.

The idea with the DIVs wasn't really good. Sorry. If you put an image between P-tags you get errors trough validating your site. I've tested the patch in #11. There seem to be no disadvantages after changing.

Committed #11, thanks!

Hi,

I've just had the same problem with 3-x.dev version. I had to change the styles in img_assist.css to :

span.inline-left  img{
  display: block;
  float: left;
  margin: 5px 5px 5px 0px;
}
span.inline-right  img{
  display: block;
  float: right;
  margin: 5px 0px 5px 5px;
}

I had the same problem as #16,
changing "span.inline-left{" to "span.inline-left img{" worked great.

Reverting to fixed, as I believe that the last follow-ups need a separate discussion (which might exist already, so please search the active issues before creating a new one, thanks).