134580 - vspace default (images separated by whitespace lack vertical gaps)

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Jamie Zawinski]

IMG has always defaulted to VSPACE=2, until Mozilla.
Therefore, many pages that looked good in NS4 look wrong
in Mozilla, because the images run right up against each
other.  For example, an HTML document that consists of
nothing but <IMG ...> tags with whitespace between them --
in NS4 and earlier, you end up with a page with a few pixels
of space between images, horizontally and vertically.  In
Mozilla, there is a horizontal margin but no vertical margin,
and it looks bad.

Yes, adding VSPACE=2 to the HTML of every page works.
No, I don't consider that a reasonable thing to have to do.

Comment 1

[Chris Casciano]

If I understand you correctly I have always thought of this as a bug with NN4.
After all why should this:

<img /><img />

Render differently then this:

<img />     <img />

Or this:

<img />
<img />


[and as a fix, if you really must have this behavior in mozilla try using img {
padding:2px 0; } in a global stylesheet rather then adding the vspace attribute
to every img tag]

Comment 2

[Hixie (not reading bugmail)]

vspace?

Whatever it is it isn't compliant...

Comment 3

[Christopher Hoess (gone)]

Huh?  vspace is a legitimate (albeit deprecated) presentational attribute. 
Anyway, we seem to be doing the right thing per CSS2; we calculate the height of
the line box from the uppermost contained box top to the lowermost contained box
bottom, that is, the top and bottom of the images.  "Line boxes are stacked with
no vertical separation."  See Chris' suggestion if you want the vertical
padding.  Resolving "INVALID".

Comment 4

[Jamie Zawinski]

Chris Casciano wrote:
>
> After all why should this:
> 
> <img /><img />
> 
> Render differently then this:
> 
> <img /> <img />

It should (and does) because the whitespace is significant:
whitespace between image tags is compressed to one space,
and rendered as such.  So the horizontal distance between
images will be increased by the width of the space char
in the current font in that case.

But anyway, that's not what I was talking about.  I think
the horizontal spacing is correct.  It's the vertical spacing
that I'm complaining about.

The IMG tag has VSPACE and HSPACE attributes which are the
extra margin to leave around images:
http://www.w3.org/TR/REC-html32#img
It is analagous to CELLPADDING in table cells.

These values have always defaulted to 2 in Netscape
(and I believe, also in MSIE.)

Mozilla appears to be defaulting them to 0.

As you see in that URL, the HTML 3.2 spec said,
> By default VSPACE is a small non-zero number

Christopher Hoess wrote:
>
> we calculate the height of the line box from the
> uppermost contained box top to the lowermost
> contained box bottom, that is, the top and
> bottom of the images.  "Line boxes are stacked with
> no vertical separation." 

What you have quoted says "do not insert extra whitespace
between lines."  In other words, no additional leading.

It seems clear to me that the HSPACE and VSPACE of an
image are *within* the "contained box".  And, in fact,
Mozilla treats them that way: if you explicitly specify
a larger VSPACE on an image, the line height increases
accordingly, if that IMG increases the overall height
of the line.

So this is not about line leading, or about Mozilla's
interpretation of the HSPACE and VSPACE attributes: 
this is merely about the fact that you have changed the
default HSPACE and VSPACE values on the IMG tag from 2 to 0.

You *do* still support HSPACE and VSPACE, and you support
them properly -- you've just changed the default.

So no, this is not INVALID.  Unless you can show me some
spec that says that HSPACE and VSPACE are required to 
default to 0.

I'm not interested in working around this Mozilla regression
by adding style sheets: I create all my web sites in HTML 3.2
so that they remain portable.  I'll start using style sheets
when NS4 is no longer showing up in my HTTP logs.

Comment 5

[Christopher Hoess (gone)]

Resummarizing.  cc'ing fantasai, who's familiar with our UA stylesheets.

Comment 6

[Dawn Endico]

i think this is a quirks issue. cc'ing dbaron

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could someone attach (text/html attachment) an example of what we're talking
about so we know what to argue about?  I think there are at least 3 different
issues in this bug (default padding/margin, treatment of whitespace between
images, and the interaction of space for descenders and baseline-alignment of
images).

My guess at what the original report is saying is that we should be leaving
space for descenders of whitespace (collapsed to a space character) that
horizontally separates images (and perhaps ascenders as well, if the images are
small enough).  That's conceptually very different from a default VSPACE.  I'd
have to go back and reread bug 24186 (that was a long time ago), but I think
there might have been the issue (this one?) that I couldn't figure out how to
make perfectly backwards-compatible in that patch because I couldn't figure out
exactly what Nav4.x and IE were doing.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: dbaron's guess at a testcase

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a bunch of relevant testcases with images

This is a group of testcases that should show what the problem is.

As my memory of bug 24186 returns, I think the inconsistency that we don't
imitate is this:  Nav 4.x includes the height of metrics for horizontal chunks
of whitespace separating images, but it doesn't do the same for chunks of
whitespace separating inline elements of smaller font size.  I think 4.x also
had a quirk that drove web authors crazy where it included the metrics for
either leading or trailing whitespace even when that whitespace wasn't
displayed horizontally (or something like that).

This requires another set of testcases...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase looking at leading/trailing whitespace issues

This looks at leading/trailing/internal whitespace for text and for images. 
The inconsistency that I remembered was a figment of my imagination, at least
based on these tests.  Nevertheless, web developers hated this (for the same
reason they hate bug 22274, so I don't sympathize all that much).

Could some Windows user tell me what WinIE6 does on this testcase?  How about
MacIE5?

What Nav 4.x does, for those who don't have it, is that it consistently (for
both text and images) uses the metrics from intermediate or trailing
whitespace, but ignores the metrics of leading whitespaces (which is compressed
anyway).

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that makes us almost compatible with Nav4.x

This patch makes us almost compatible with Nav4.x on the testcases in this bug.
 The difference is that we trim trailing whitespace as well as leading
(somewhere else), as we probably should.

Any word whether this moves us closer to or farther from WinIE?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: revised patch

Pulled the loop invariant out of the loop and added comment.

FWIW, this messes up the bugzilla footer, although not quite the way it's
messed up in 4.x (the extra space is on the bottom instead of the top).  I
wonder why...

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, the only bugzilla footer that's messed up is the one on the "Create
Attachment" page.  I wonder why they're different.

Comment 14

[Jamie Zawinski]

If there are any bugs here related to *horizontal* spacing,
then they are not the bug I reported.  The bug I reported
is this and only this: "VSPACE used to default to 2.  Now
it defaults to 0."  Here's a test case showing what I'm
talking about.

Comment 15

[Jamie Zawinski]

Attachment: VSPACE test cases

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 77132 [details]
VSPACE test cases

In my copy of Netscape 4.78 for Linux, Test 1 and Test 2 are displayed
differently: Test 1 has extra space.

Comment 17

[Jamie Zawinski]

> In my copy of Netscape 4.78 for Linux, Test 1 and Test 2 are displayed
> differently: Test 1 has extra space.

Hmm, right you are.  Netscape 4.78 and 3.02 display the same way.

I'd be satisfied with either outcome (more space or less).
The real problem is when there is no space at all, which
totally changes the look of the page.

Comment 18

[Bradley Baetz (:bbaetz)]

dbaron: The <table> footer being different on attachment pages is because that
page has a stylesheet applying to <td>. Theres a bugzilla bug on it somewhere.

Comment 19

[R.B.]

It seems that the behaviour of Mozilla in "quirks" mode is still different from
NS4 and IE. Two cases are OK:

-- <img />-s in table rows, no text, no whitespace: no space between rows;
-- <img />-s and text in table rows (a single "." is enough): space for
descenders is left.

Third case does not match:

-- <img />-s and whitespace only in table rows: no space between rows! (But
space between images in the same row where appropriate, of course.)

NS4 and IE leave "descender space" in this case too.

Is this intentional? I had a quick look at bug 22274 and links given in it, but
couldn't find an answer.

Comment 20

[R.B.]

Hmm, seems that my last comment was already discussed in Comments #9 - #12.
Sorry for bugmail avalanche.

Comment 21

[Christine Hoffman]

Changing QA contact

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, taking the bug, since I've been running with the patch for over a month and
haven't noticed any problems, other than the figment of my imagination in
comment 12 and comment 13 which comment 18 correctly notes is a correct
rendering of the HTML/CSS in question.

Comment 23

[Marc Attinasi]

Comment on attachment 77116 [details] [diff] [review]
revised patch

r=attinasi

Comment 24

[Chris Waterson]

Comment on attachment 77116 [details] [diff] [review]
revised patch

sr=waterson

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk 2002-04-30 17:42 PDT.  (Leaving open since I may request
1.0 branch approval in a week or so.)

Comment 26

[Jacob Kjome]

David,

You first and second testcases cause Mozilla to be extremely sluggish.  Very
unresonsive.  This is not so in IE5.5.  Is this a known bug?  Is this caused by
your checking?  Should it be reported as a new bug?

I am using build 2002050108 on Win2k (SP2sr1)

Jake

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In so far as the patch changed control flow, it would have sped things up (since
we would break out of the loop sooner).  I don't see anything sluggish (on
Linux) when loading either of the testcases.  What's slow?  Loading?  Scrolling?
 Resizing the window?  Painting?  All of the above?

This sounds like something that should be filed as a separate bug.

Comment 28

[Jacob Kjome]

all of the above is slow.

It seems to make the rest of my system sluggish because any action taken on 
Mozilla spikes the cpu on the first two testcases:

http://bugzilla.mozilla.org/attachment.cgi?id=77062&action=view

Especially this one
http://bugzilla.mozilla.org/attachment.cgi?id=77087&action=view

I think I'll re-test with tomorrows build since that one will have the fix for 
bug 102321.

Jake

Comment 29

[Scott Kester]

I think this patch is causing layout issues on sites that use 'shim' images to
control layout spacing.  I found the problem on http://www.weather.com.  What
should be a 2 pixel space at the top of the page is expanded to around 10
pixels.  This seems to be caused by the comment lines above the shim image.

I have created two test cases using shim images, one page with comments, and one
without.  The case with comments shows the 2 pixel space being expanded to many
more.  The cases render identically in NS4 and IE6, and Mozilla builds prior to
20020501.

Comment 30

[Scott Kester]

Attachment: Test case showing problem with shim images and comments

Comment 31

[Scott Kester]

Attachment: Shim test case without comments

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that fixes case of comments at the beginning

This fixes the case of comments at the beginning.  Comments at the end would
actually be a little harder.  I should test to see what IE does.  (Nav4 puts
extra space because of comments at the end, but I suspect we probably
shouldn't.  See comment 11.

Comment 33

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that fixes case of comments at the beginning

oops, wrong patch

Comment 34

[Jacob Kjome]

David,

The problems I mentioned in comment 26 and comment 28 seem to be fixed.  I am
guessing they were fixed by the fix for bug 102321.

Jake

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: more thorough version of testcase looking at leading/trailing whitespace issues

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I realized that the patch for bug 68489 that I had in my tree was messing up my
testing of this patch the previous time around.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: more thorough yet

Comment 38

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase

Comment 39

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 83883 [details]
testcase

(Y=gap, N=no gap)

YNYYN NNYY YNYYN NNYY YNYYN NNYY YNYYN NNYY - NN 4.78 Linux
YNYYN NNYY YNYYN NNYY NNNNN NNNN NNNNN NNNN - Mozilla 1.0rc2 Linux
YNYYN NNYY YYYYN NYYY YNYYN NNYY YYYYN NYYY - Mozilla trunk Linux

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

YNYYN NNYY YNYYN NNYY YNYYN YNYYN NNYY YNYYN NNYY - WinIE 5.5

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Ignore the extra group above.)

NNNNN NNNY NNNNN NNYY NNNNN NNYY NNNNN NNYY - MacIE 5

Comment 42

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that makes us compatible with NN4 and WinIE 5.5

This is just attachment 82020 [details] [diff] [review] plus comment changes.

Comment 43

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a few *more* testcases

Yes, more.

The patch even seems to work correctly on the last one.  I've looked at too
many of these already today to understand why.

Comment 44

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a few *more* testcases, take 2

No wonder we passed it -- I got the testcase wrong.  So I still need to figure
out how to fix the last 2 cases in this one.

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a few *more* testcases, take 2

No wonder we passed it -- I got the testcase wrong.  So I still need to figure
out how to fix the last 2 cases in this one.

(That last one was just the wrong file.)

Comment 46

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that makes us compatible with NN4 and WinIE 5.5

This patch uses |nsIFrame::IsEmpty| rather than using an incorrect
approximation for emptiness.

Comment 47

[Chris Waterson]

Comment on attachment 83904 [details] [diff] [review]
patch that makes us compatible with NN4 and WinIE 5.5

sr=waterson

Comment 48

[Marc Attinasi]

Comment on attachment 83904 [details] [diff] [review]
patch that makes us compatible with NN4 and WinIE 5.5

r=attinasi

Comment 49

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in 2002-05-16 12:44 PDT.

This isn't going to make 1.0, so marking fixed.

Comment 50

[Jacob Kjome]

David WinIE6 looks very different than Mozilla in
http://bugzilla.mozilla.org/attachment.cgi?id=83896&action=view under the header
"A few more".

In IE6, line of text that comes after the image is right up flush under the
image above it.

so...

Text with initial whitespace containing a span: 
[image]
TextText
[image]

Text with initial whitespace containing a span: 
[image]
TextText
[image]

Where Mozilla has lots of space between the image and the following text.

Also, the image testcases under the header "Testcases where whitespace a single
text node and td has explicit span" only like like IE6 when in Quirks mode. 
When a Strict doctype is added, they look different...especially the last one
where there ends up being a space underneath the side-by-side images.

Is this really compatible, or did IE6 just go off the deep end?  I'll have to
wait to check IE5.5 at work.

Jake

Comment 51

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I only checked the patch in this afternoon.  Try tomorrow's build.

Comment 52

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, and we're only talking about quirks mode here -- all of the changes for this
bug are quirks-mode only.  In standards mode we actually (gasp) follow the
standards (hence bug 22274).

Comment 53

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This caused bug 155333.