40596 - input type=text form controls with width:auto are not rendering correctly in table cells

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[djoham]

From Bugzilla Helper:
User-Agent: Mozilla/4.7 [en] (X11; I; Linux 2.2.14-15mdk i686)
BuildID:    2000052508

according to the CSS1 spec (www.w3.org/TR/REC-CSS1 section 5.5.23) elements with
width:100% should defer to their parent element's width.

When an input value is placed in a cell, I expect the parent element would be
the cell itself. Using width:100% in the cell should size the input element to
100% of the cell width. Mozilla is not doing this. Rather, Mozilla is rendering
the input at its "default" size and overriding the width of the cell.

Reproducible: Always
Steps to Reproduce:
1.Open my test case. 
2.
3.

Actual Results:  Note that the second table's right hand cell width has been
overridden by the input field. I've tried input type=button as well, with the
same result.

Expected Results:  I would expect that the input box would be 100% of the cell
width.

This bug might be related to bug 40283

I consider this a major bug since tables are used heavily on the web as layout
managers. Major work arounds will have to be employed if this is not fixed.

IE5 renders my test case as I would expect.

Comment 1

[djoham]

Attachment: test case

Comment 2

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

'width: 100%' is not the same as 'width: auto'.  Probably width: 100% inside a
table cell should result in infinite width (but really the spec should be
changed).  This stuff (percentage widths in table cells) isn't defined very
clearly in CSS2.

Is this specific to form controls or does it occur with all elements in table
cells?

Comment 3

[djoham]

I tried auto as well, but I got the same result when I don't specify a DTD. When
I specify the strict DTD, width: auto actually renders worse that width: 100%.

I haven't looked at the CSS2 spec, but it seems that the CSS1 spec (outlined
below) is rather clear in its meaning (at least to me :).

I'll include a two test cases that compare and contrase width: 100% and width:
auto both with and without the strict DTD.

I'm not sure what you mean on your last question. What else besides a form
element would you put in a table cell? The only thing that I can think of would
be an image, but bug 39901 prevents me from testing that.

Comment 4

[djoham]

Attachment: compare/contrast width: 100% to width: auto with strict DTD

Comment 5

[djoham]

Attachment: compare/contrast width: 100% to width: auto w/o strict DTD

Comment 6

[rods (gone)]

I think this is a dup

Comment 7

[Bernd Mielke]

Putting David Baron on CC list as he has filed bug 41306, that looks close related.

Comment 8

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

No, this is a separate issue.  It's not completely clear that it's a bug, and it
looks to me to be more complicated than bug 41306.  But marking confirmed.

Comment 9

[karnaze (gone)]

Rod, when I use a <div>x</div> instead of the <input>, the 20% is honored. The 
<input> must be needing some min width (like 21 characters).

Comment 10

[Kevin McCluskey (gone)]

Setting Milestone to M17.

Comment 11

[Mike]

M16 has been out for a while now, these bugs target milestones need to be 
updated.

Comment 12

[rods (gone)]

setting to M17

Comment 13

[rods (gone)]

The fix is checked in and ifdef'ed in two places, nsGfxTextControlFrame2.cpp and 
nsBoxFrame.cpp

Comment 14

[Kurt George Gjerde]

I tried this on Netscape 6 Prev 1 (win2k) and it renders ok. Width 100% in INPUT
spans the entire cell unless it needs more space specified by the SIZE
attribute. This is the same way as MSIE 5 does it.

Comment 15

[Kevin McCluskey (gone)]

Marking nsbeta3+

Comment 16

[rods (gone)]

fixed

Comment 17

[rods (gone)]

I reopening because of two issues. The GfxText is not setting the maxelementsize 
for the height correctly and this makes table layout spew warnings. And I didn't 
quite get the reflow sizing correct. If the computed size is uncontrained but 
the available size was set the GfxText wasn't using it and it should have been.

Plus, I introduced a regression.

Comment 18

[rods (gone)]

fixed

Comment 19

[David Hyatt]

use hyatt's account from ftang(sheriff of today) . I ask hyatt back out 1.120 of 
nsBoxFrame.cpp to fix 40596. rods, your fix cause a lot of assertion in 
mailnews. Please re-exam your fix. Thanks

Comment 20

[David Hyatt]

still ftang- sorry, it should be r 1.120, not r 1.119.

Comment 21

[Marc Attinasi]

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

Comment 22

[rods (gone)]

fixed

Comment 23

[rods (gone)]

refixed

Comment 24

[rods (gone)]

reopening

Comment 25

[rods (gone)]

I reopened because I had to remove some of the code that fixed this bug, the 
reflow for nsBoxFrame cannot take into account availableSize, it must size 
itself to either the computedSize or its preferredsize, and it cannot fall back 
onto the availableSize.

The real issue with this bug is the cell or the block in the cell is not 
computing the computedSize when the cell is a percetage width. The GfxText would 
correctly obey the computedWidth if the cell computed it correctly instead of 
leaving it unconstrained.

reassigning to attinasi - its actually a table bug. This was plused before 
because I had a fix, thhe plus may need to be cleared, except I think this is an 
important bug to fix.

Comment 26

[rods (gone)]

This is noticable on www.cnn.com in the CNN on TV part in fact I think it causes 
the button to be rendered incorrectly.

Comment 27

[Marc Attinasi]

Sending to karnaze and CCing buster in case he believes it is really a block 
inside the table problem.

Comment 28

[leger]

PDT agrees P1

Comment 29

[karnaze (gone)]

Buster, the only problem that I am seeing now is with the 3rd table in the 2nd 
and 3rd attachments, where the text field spills out of the table cell. I've 
included part of a "dump frames" of the 3rd table in the 2nd attachment below. 
The cell's block is narrower than the gfx text frame. 

 TableCell(td)(1)@02822028 {4749,0,1191,390} [state=00000004] sc=033F0D50
  <Block(td)(1)@02822088 {30,30,1131,330} [state=001c000c] sc=03405C80(i=2,b=0)
   <line 0282246C: count=2 state=inline,clean,NOT Impacted,[0x1020] 
{0,0,2460,330} 
    <Text(0)@028220D4[0,49,T]  next=02822110 {0,270,0,0} [state=51000024] 
sc=034158A0
     <"\n">
     nsGfxControlFrame2@02822110 next=02822430 {0,15,2460,300} [state=20c04024] 
sc=034170B0<

Comment 30

[buster]

Attachment: patch for remaining text field sizing problem

Comment 31

[buster]

Pat, can I have an "r"?  I'd like to solve the bug...

Rod, does the patch look good?  The point is to treat width:auto as "default" 
width, and also to ensure that the control returns a legal value for it's max 
element size.  It was returning -1 which is why line layout was confused.

cc-ing evaughan since part of the fix involves nsBoxFrame.cpp

Comment 32

[buster]

changed the subject to reflect the remaining problem.
marked [fix in hand], waiting for review from rod and eric

Comment 33

[buster]

Attachment: better, safer fix that does not change nsBoxFrame.cpp at all. All changes in nsGfxTextControlFrame2.cpp.

Comment 34

[buster]

Thinking about my first fix, I didn't like the fact that we were using -1 as a 
magic number, or that nsBoxFrame was involved at all.  So this new fix is scoped 
entirely within the text control frame itself.

Note that some of the text in the attached test cases is wrong.  The claim is 
that all 3 tables in each test case should lay out the same.  But the case of 
width:100% and width:auto are very different.  For an inline replaced element 
like a text control, the former means "take up all available space", while the 
second means "be your intrinsic (default) size."  So the second case effectively 
sets a fixed width on the text control that is dependent on the font being used.  
If this default width is greater than 80 pixels (the width of the second TD, 20% 
of 400px), then the second TD will expand to accomodate it.  Check it out in IE 
5.5, it does the same as mozilla does with my fix.

I just got verbal r=rods. 

Comment 35

[buster]

r=rods, fix checked in

Comment 36

[gerardok]

massive update for QA contact.

Comment 37

[djoham]

Drat.

This seems to have regressed on the latest daily builds. The problem with the
CNN TV stuff rendering is back on both Linux and Windows for the September 7th
Daily builds. The Linux build number is 2000090608.

Using the smoking gun theory of debugging, I think I may know why. Version 1.90
of nsGfxTextControlFrame2.cpp introduced a new function -SetInitialValue()- into
the same area of code where the fix for this bug was placed. The
SetInitialValue() code was committed to CVS after the fix for 40596 was checked
in in version 1.86.

I seem to recall once this week being very happy seeing a CNN TV box that
rendered correctly so I believe at one point everything was well with the world
as far as 40596 is concerned. That also leads me to believe a change after 1.86
is the reason for the regression.

Any thoughts?

At the moment, I'm re-opening the bug due to regression.

David

Comment 38

[Michael La Guardia]

Per 8/30 comment, adding PDTP1 to status.

Comment 39

[buster]

[e-mail]:
Sorry, you lost me.  What does this bug have to do with the CNN web site?  Why
was this bug re-opened?  Please be very specific, we're in the midst of a big
push towards beta3 and we're a lot more likely to get this right if we know
precisely what is going on.  Thanks!

lorca, any idea what's going on here?

Comment 40

[buster]

the CNN bug (really bad source, not a bug in mozilla) is covered by bug 48057.  
What is the relevance to this bug?

marking fixed again.  please contact me directly if you disagree, so we can 
reach an agreement about what's going on here.

Comment 41

[djoham]

Sorry guys. I was going off of Rod's comment from  8/24 that mentioned CNN. 
At one point, Mozilla was working with CNN (I swear I saw it!). This was roughly 
the same time that this bug was fixed. Since it later broke, I assumed 
regression.

Carry on. Carry on. Nothing more to see here..... :)

Comment 42

[Loco]

Fixed (regressed to PR2 Mac OS 9) in 2000-09-13-13.

Comment 43

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

SPAM. HTML Element component is deprecated, changing to Layout component. See
bug 88132 for details.

Comment 44

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

Come on Bugzilla, you can do it...