273361 - mew should not be smaller than fixed min width

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Roger Oscarsson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041205
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041205

When viewing the url one is immediatly sees that something is wrong. This url is
temporary and is just a frameset of different kinds of errors. Please use the
attached html file instead.

The most obvious error is the yellow text with the red background. It is outside
it's designated cell. This seems to be related to setting min-width. Removing it
 keeps the text in it's cell. This error could possible be a completely
different error and should then given it's own bug id.

The other error is the fact that setting the width of a table cell doesn't seem
to affect the width at all.
According to http://www.w3.org/TR/CSS21/tables.html#width-layout under
"Automatic table layout".

Quote:

    Calculate the minimum content width (MCW) of each cell: the formatted
    content may span any number of lines but may not overflow the cell box.
    If the specified 'width' (W) of the cell is greater than MCW, W is the
    minimum cell width. A value of 'auto' means that MCW is the minimum cell
    width.

End Quote

Which I interpret as any given width to a table cell is the same as minimum
width. (Which is somewhat strange since there exists min-width in CSS.)

The other problem is the fact that it changes rendering depending on whether
there are spaces between "FOO" and "BAR" or non-breaking spaces. If the "FOO
BAR" with red background and yellow text has a non-breaking space it the inner
table will get larger but the other "FOO BAR" still is on two lines. If it's the
 other "FOO BAR" that gets the non-breaking space it will also expand the table
(to what looks like) the same width.

These are the errors I can see. There are some problems with font size but it
seems to be almost the same problems as the space and non-breaking space
problem. I might of course be wrong.


Reproducible: Always
Steps to Reproduce:
1.View the url
2.Be appalled by how wrong it is rendered.
3.Report the problem! :-)



Expected Results:  
Compare to how Safari does it. I'm not saying that Safari get's it right but it
is better!

Same problem with Firefox 1.0  under MacOS 10.3.7.

The reason for my discovery is that I wanted to make sure that a table was at
least as wide as the picture (with some padding).

My first solution was to set the width on the table cell. Mozilla completely
disregarded that. My second solution was to set min-width on the table cell. As
expected that didn't work. Than I tried to set width on the content. That sort
of worked but then the text failed to be always centered (which is correct since
width of the box was set). Then I tried min-width on the content and got horror. 

PS. Sorry for my rather incoherent english

Comment 1

[Roger Oscarsson]

Attachment: Test example which shows the error(s)

It has a external reference to an image which probably is not available. The
image is not important. It just makes the errors more apparant.

Comment 2

[Bernd]

the div does not report the min width as mew as it should but reports rather the
height.


block 034DB284 r=0 a=UC,UC c=UC,UC cnt=1346
 text 034DB350 r=0 a=UC,UC c=UC,UC cnt=1347
 text 034DB350 d=480,228 me=480
 br 03541A68 r=0 a=UC,UC c=UC,UC cnt=1348
 br 03541A68 d=0,0 me=0
 text 03541A94 r=0 a=UC,UC c=UC,UC cnt=1349
 text 03541A94 d=816,228 me=396
block 034DB284 d=1476,480 me=480

Comment 3

[Bernd]

its of course not the height but the mew of the first text sorry

Comment 4

[Bernd]

Attachment: testcase min width vs mew

Comment 5

[Bernd]

This is problem with a couple of frames. The testcase is not complete but it
shows the issue. Just a explanation how it works, the second cell has 100% width
specified this will expand the cell as much as possible and shrink as small as
possible the first cell. If a frame reports a wrong MEW
(http://www.mozilla.org/newlayout/doc/block-and-line.html#maxElementWidth) the
first cell will shrink to much and the frame which honours the min-wdith will
then overflow.

Comment 6

[Bernd]

Attachment: patch for blockframe

Comment 7

[Roger Oscarsson]

(In reply to comment #6)
> Created an attachment (id=168334)
> patch for blockframe
> 

I have three questions.

First:
Wouldn't it be beetter if "maxElementWidth" was set to "computedMinWidth"
since it has the same value (computed just some lines before).

Secondly:
The patch looks a suspiciously narrow. It very specific conditions (whether the
width as been given in percentages or not). And looking just some lines before
you have the same test for "aState" having flag "BRS_COMPUTEMAXELEMENTWIDTH"
set and there it also play with "maxElementWidth" (but the other way around).
My feeling is that they should be related in some way.
Please remember I have NO idea what I'm talking about here since I don't have
no knowledge of the mozilla code. I just looked at it from above and thought
it looked suspicious. 

Third:
Are you sure this fixes the problem with the table cell ignoring a set width?
It just seems strange that this fix should fix both since they behave so
completely different.

Comment 8

[Roger Oscarsson]

Attachment: Example showing problems with table cell ignoring set cell width

Comment 9

[Bernd]

Attachment: more frames that have a minwidth mew issue

Comment 10

[Bernd]

Attachment: revised patch

Comment 11

[Bernd]

just a reminder: I use bugzilla as patch backup system, only patches with review
requests are somehow final.

so I am trying to answer your questions:
1) your are right I think the patch looks nicer, thanks.
2) it is suspiciously narrow as it fixes only blocks, but for blocks I believe
its correct now
a) only if the BRS_COMPUTEMAXELEMENTWIDTH flag is set we are required to compute
the MEW.
b) only if the MEW is smaller than the computedminwidth we need to increase
towards the computedminwidth
c) if the minwidth is a percentage base property it should not influence the MEW
as it might shrink down to 0 if the parent is only small enough. Please compare
this with
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsHTMLReflowMetrics.h#208
where we don't use a percentage based computed width for the MEW as this will
blow up incremental reflow this was fixed in bug 246999.
3) table cell widths are more a rough guideline, so this bug is about if you
specify a minwidth on a descendant of a table cell the table cell should be
large enough to prevent overflow of this child if it is only as large as
specified in the min width.

Comment 12

[Bernd]

>Are you sure this fixes the problem with the table cell ignoring a set width?
This another issue, btw. we render the testcase as IE and opera. Have you seen
any browser that differs in the rendering? I dont believe that it is bug at all
(with the 100% the thing is anyway overconstrained allready). The overflow
certainly is one and will be fixed.

Comment 13

[Bernd]

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

David, I hope this is the correct way. Or should that go above where the MEW is
computed? 

(I will remove the linefeed at the end)

Comment 14

[Roger Oscarsson]

(In reply to comment #12)
> >Are you sure this fixes the problem with the table cell ignoring a set width?
> This another issue, btw. we render the testcase as IE and opera. Have you seen
> any browser that differs in the rendering? I dont believe that it is bug at all
> (with the 100% the thing is anyway overconstrained allready). The overflow
> certainly is one and will be fixed.

I thought I tried it in Safari, but it seems I that didn't. It has the same
behaviour. It disregards width on table cell. BUT! it works with min-width and
max-width. Since I think that setting min-width and max-width to the same thinh
should be the same as setting width, I think Safari does something wrong.

Why do I think it is a flaw. First the standard seems to say it (see reference
in original bug report). And secondly, it cares about the width of the actual
table (see last example in https://bugzilla.mozilla.org/attachment.cgi?id=168045
). I think it should care about both or care about none just for the sake of
consistency. Since caring about none would be rather limiting (in table design)
I think that it should care about both.

PS. I really should try to bug report to the Safari people or possibly the KDE
people.

Comment 15

[Bernd]

If the problem that you see is so cross browser, w3c-style might be more
appropiate.I don't care much about bugs in other browsers, we have enough of
them allready.

Comment 16

[Roger Oscarsson]

(In reply to comment #15)
> If the problem that you see is so cross browser, w3c-style might be more
> appropiate.I don't care much about bugs in other browsers, we have enough of
> them allready.

Sorry to be dumb, but what do you mean with "w3c-style"?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Bernd, doesn't this need to land?

Roger, the [e-mail] is the place to bring up issues with the spec or
with user-agents all acting in a consistent way different from the spec...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Er... sorry about that.  And I didn't realize dbaron had just reviewed...

Comment 19

[Bernd]

yes this needs to be landed but landing without approval when the tree is closed
isn't good style :-) 

Comment 20

[Bernd]

The fix for the block code got checked in, I will also post a patch for the
other frames that are indicated in attachment 168500 [details].

Comment 21

[Ryan VanderMeulen [:RyanVM]]

This bug WFM. Bernd, are there issues here which still need fixing or can this be resolved?

Comment 22

[Bernd]

In a Gecko/2007123105 Minefield/3.0b3pre build the legend and the submit image case are not at least 400px wide in attachment 168500 [details] , the hidden is also not but that is the sense of hidden.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

dholbert's been doing a lot with these kind of table layout issues in other bugs, I wonder if this one would be easy to pick up along with the others.

Comment 24

[Daniel Holbert [:dholbert]]

Attachment: testcase for <input type="image"> (quirks mode)

(In reply to comment #22)
> In a Gecko/2007123105 Minefield/3.0b3pre build the legend and the submit image
> case are not at least 400px wide in attachment 168500 [details]

Here's a reduced testcase for that submit image situation.

Comment 25

[Daniel Holbert [:dholbert]]

Requesting blocking for the layout regression shown in attachment 295649 [details] and described in Comment 22 & Comment 24

Comment 26

[Daniel Holbert [:dholbert]]

Attachment: testcase for <input type="image"> (standards mode)

Comment 27

[Daniel Holbert [:dholbert]]

(In reply to comment #25)
> Requesting blocking for the layout regression shown in attachment 295649 [details] and
> described in Comment 22 & Comment 24

Oops -- Bernd pointed out that this isn't actually a regression.  Cancelling blocking request. 

I'd been confused because FF2 does something different (which I thought was correct behavior) from FF3 in Quirks mode, as shown by attachment 295649 [details].

However, as shown by attachment 295813 [details], FF2 and FF3 are *both* broken in Standards mode.  (Not sure why the quirks-mode behavior has changed in FF3, but that's probably a separate issue -- maybe we should file a new bug for that?  Or is that change desired?)

Comment 28

[Bernd]

the fieldset legend seems to be what we want:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/forms/nsFieldSetFrame.cpp&rev=3.166&mark=351,352#347

Comment 29

[Bernd]

Attachment: replaced image min width

the submit type image is the consequence of alternate text for images which does not honor min width.

Comment 30

[Bernd]

after a short reading of http://www.hixie.ch/advocacy/alttext I think we should close this bug as WFM.

Daniel, do you agree?

Comment 31

[Bernd]

marking as wfm, please reopen if you disagree.