58899 - no margin around floated images and spans (and probably other inlines)

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Miloslaw Smyk]

From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (X11; U; Linux 2.2.17 i686)
BuildID:    2000102608

In the page at http://amiga.com.pl/ all other browsers add a small transparent
gap around images of posters faces. It is especially evident between right edge
of the images and left edge of the news' text. In Mozilla, there is no gap.

Reproducible: Always
Steps to Reproduce:
1. Just visit http://amiga.com.pl and take a look


Expected Results:  I think there should be a gap, as all other browsers do this.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 2

[Mats Palmgren (inactive)]

Confirming bug, build 2000-11-02-04 on Windows NT4 sp6.
Floating images has a margin in older browsers.
IE5 and Communicator 4.75 has a 3px margin. Opera 4.02 has a 2px margin.

Adding the following to html.css seems to fix it:

img[align] {
  margin: 3px;
}

Comment 3

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 4

[Mats Palmgren (inactive)]

It's only horizontal aligns that should have a margin. Here is a better patch:

img[align=left], img[align=right] {
	margin-left: 3px;
	margin-right: 3px;
}

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Please triage.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Marc, it seems like you have done most changes in html.css so I am giving this
to you...

Comment 7

[Hixie (not reading bugmail)]

We used to have this in our html.css (check lxr) -- why did we remove it?
I would guess it's because it made width:100% fail (since CSS doesn't include
the margin in the width).

Comment 8

[Marc Attinasi]

Hmm. I'm not sure which is better, the margin or the width:100% correctness.

Could this qualify as a quirk? Ian, do you know if the HTML spec indicate that
there should be a margin, or is this just a compatibility issue?

I'm agnostic on this one: I can easily add the rule, but how important is it?

Comment 9

[Hixie (not reading bugmail)]

HTML says nothing about it, personally I would not check it in and if we do it
would be strictly a quirk mode thing. This doesn't seem to affect any major
sites does it? And the workaround is pretty simple (use CSS)...

Comment 10

[Marc Attinasi]

If Netscape4 and IE put the margin in then we probably want to as well, but it
would be nice to know if this is really something that is relied upon widely on
the net. Any other sites that exhibit this?

Accepting for now, but might be a WONTFIX in the end.

Comment 11

[Mats Palmgren (inactive)]

15 minutes of surfing and I found 5 sites having this problem, 1 top100.

1. http://www.segfault.org/
2. http://www.zdnet.com/zdnn/ (the images in the "Commentary" column)
3. http://internetworld.idg.se/ (image left of "Testa dig själv")
4. (top100) http://www.amazon.com (image left of "Visit our British store") 
5. http://www.fool.com/ (first 3 images under the search form have align=right)

I am sure I have seen this at other sites too.
Ian said: causes potential std comp problems
How come, if this is fixed with a rule in quirk.css? and also, floating images
with a percentage based width must be extremely unusual...

Comment 12

[Marc Attinasi]

Thanks, Mats. I'll add this to Quirk mode.

Comment 13

[Marc Attinasi]

Attachment: Patch to quirk.css to give a margin to floated images

Comment 14

[Marc Attinasi]

One issue with the patch: it only takes care of images floated with the align
attribute. If the image is floated with css using a rule like
'img.foo { float: left; }'
then it needs to get the margin too. Actually, Nav4 gives the CSS-case twice the
margin of the align case, but IE makes them have the same margin, which seems
more reasonable.

I think we need to fix this such that we provide the margin like IE does it, in
Quirks mode only of course.

Comment 15

[Marc Attinasi]

Attachment: testcase showing a floated image using the align attribute and CSS float property

Comment 16

[Marc Attinasi]

Attachment: Image for testcase

Comment 17

[Marc Attinasi]

Attachment: Testcase again with images this time. Ignore previous testcase, sorry.

Comment 18

[Marc Attinasi]

This looks like a general problem with margins on floated inlines in Quirks mode.

I left-floated a span and put some text next to it: Mozill aapplies no implicit
margin, whereas IE puts a 3px margin, just like it does for images.

cc'ing buster for his thoughts: should we provide a margin on floated inlines in
Quirks mode?

Comment 19

[Marc Attinasi]

Attachment: Testcase with floated span and floated image: no margins

Comment 20

[Hixie (not reading bugmail)]

Marc: If the author is using CSS in the first place, then we should not bother
doing any 'magic' with margins. Since they are setting the 'float' property they
quite clearly can also set the 'margin' property.

Hence I think a fix that only did things for the HTML case would be best, IF we
do this at all.

Issues I have with the patch: Do we really want a left margin on images floated
left? Or only a right margin? Should the rule really be two separate rules?
Also, please use a syntax like the rest of the file for the comments, "(b=" not
" (bug " and prefix the header with "Quirk: ". Consistency makes the file look 
more professional. :-)

Comment 21

[Marc Attinasi]

I'm in agreement with you, Ian, about limiting the quirk to the non-CSS case. It
is bothersome though that IE and Nav both apply margin-magic to CSS-floated
inlines, but it has not been show to be a very important quirk (i.e. no top100
sites exhibiting bad layout).

Also, I went ahead and changed the new rules to provide the margin only to the
side the image is floated to (2 rules now) and I updated the formatting as you
suggested - IMNSHO the Quirk: designation is redundant, since the file is
quirk.css. A header indicating that these are quirk rules anyway would be
sufficient, but I like consistency too ;)

Comment 22

[Marc Attinasi]

Attachment: Updated patch

Comment 23

[Marc Attinasi]

Attachment: Updated patch: --- just shoot me now please ;)

Comment 24

[Hixie (not reading bugmail)]

Marc: The reason I used the "Quirk:" prefix is to clearly signal the start of
a set of rules that deal with a particular quirk, as opposed to comments on the
individual rules. This is only an issue wit the first two quirks in the file at
the moment though.

r=hixie for http://bugzilla.mozilla.org/showattachment.cgi?attach_id=22270

Comment 25

[Marc Attinasi]

*** Bug 55762 has been marked as a duplicate of this bug. ***

Comment 26

[Marc Attinasi]

Fix checked in: quirk.css

Thanks to Mats Palmgren for the rule suggestion!

Comment 27

[Marc Attinasi]

Fixed summary type - that is all ;)

Comment 28

[Hixie (not reading bugmail)]

v per testcases