Closed Bug 116909 Opened 23 years ago Closed 21 years ago

[standards] <br /> has width of 1px (table spacing broken after 0.9.5)

Categories

(Core :: Layout: Tables, defect, P2)

Product:
Core
Component:
Layout: Tables
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: megabyte, Unassigned)

Details

(Keywords: regression, testcase)

Attachments

(3 files, 1 obsolete file)

corrected testcase
1.39 KB, text/html
Details
Testcase #3
1.15 KB, text/html
Details
Proposed fix
1.08 KB, patch
Details | Diff | Splinter Review 
Sometime in early/mid-November, the layout engine was adversely affected. 
Unfortunately, I do not know the exact date; I do know that it works in 0.9.5
and does not work in 0.9.6
Go to the mentioned site.  Notice the headings that extend from the left bar to
the right.  In newer builds, there is a blank area at the right end of each
heading that splits the image in the table apart.  This was not present in
earlier builds and I cannot seem to get rid of it.  It is also not present in
other browsers.
Keywords: 4xp, regression
Does removing the doctype declaration from the document help?  This may well be 
a standards mode only issue.
Yes, you are correct.  It only has the problem in standards mode.
Bernd, how should we be rendering this per spec?
Keywords: qawanted
Summary: Table spacing broken after 0.9.5 → [standards]Table spacing broken after 0.9.5
Attached file testcase (obsolete) — 

    
    

    
  
it looks to me like a dupe of bug 22274 or at least I would wonder if this image
stuff would not suffer from it.

    
    

    
  

      
      
      
      

        Attached file
          
        corrected testcase 
        — 
      

    


  

        Attachment #62762 -
        Attachment is obsolete: true

    
    

    
  
I'm not exactly sure how it would be bug 22274 since that dealt with spaces 
underneath the image instead of beside the image.  Also, considering how long 
Mozilla did render this case correctly, I don't see how it could be the old 
bug 22274.

    
    

    
  

      
      
      
      

        Attached file
          
        Testcase #3
        — 
      

    


  
Seems like the table cell counts the <BR> as 1px wide?
Severity: normal → minor
Keywords: testcase

    
    

    
  
the one px wide <br> tag is probably a uprounding of the one twips it gets at
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBRFrame.cpp#189

    
    

    
  
Temporarily moving to future until a milestone can be assigned. 
Status: NEW → ASSIGNED
Target Milestone: --- → Future

    
  
Keywords: mozilla0.9.9

    
    

    
  
Not platform specific (seeing on Linux too).

OS: Windows XP → All
Hardware: PC → All

    
    

    
  
The source Bernd mentioned references bug 10036, but that was fixed long before
this problem ever showed up.  Is there something I'm missing?

    
    

    
  
the fix for bug 10036 was a hack, and now we pay the price for this.

    
    

    
  
But what was it that actually made the hack apparent?  Mozilla worked properly
in this respect for over a year without the problem after that hack was entered.

    
  
Keywords: mozilla0.9.9 → mozilla1.0, 
          
            nsbeta1

    
    

    
  
nsbeta1-. Engineers are overloaded with higher priority bugs.
Keywords: nsbeta1 → nsbeta1-

    
    

    
  
br{display:block;} is a work-around.

    
  
Priority: -- → P2

    
    

    
  
Too bad display:block severely breaks NS4.x
URL: http://www.swva.net/

    
  
Summary: [standards]Table spacing broken after 0.9.5 → [standards] <br /> has width of 1px (table spacing broken after 0.9.5)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed fix
        — 
          — Splinter Review
      

    


  
This patch simply removes the hack for bug 10036, which is apparently not
needed anymore.

    
  
Keywords: mozilla1.0, 
          
            nsbeta1- → mozilla1.0.1, 
          
            nsbeta1, 
          
            patch, 
          
            review

    
    

    
  
Did you run the layout regression tests, this is a must. Please report back the
failures of these tests. No 'r' without 'r'test. ;-). How does the patch
influence the empty table cells in quirks and standard mode?

    
    

    
  
Um, no.  The fix of bug 10036 is a happy coincidence; the real reason that code
is there is so that the <br> is included in the line-height calculation in
nsLineLayout::VerticalAlignFrames, apparently...

David, do you know what the deal is here?  It looks like the nsLineLayout code
would not ever trigger for <br> now (for one thing, I don't think the
emptyContinuation bool will be true for a <br>).  So we should be able to remove
that hack safely....

Also, setting the width non-zero in both modes is a little odd if we're only
going to ignore the line-height on the <br> in quirks mode...

    
    

    
  
emptyContinuation would never be true if the BR frame were ever a span (the
linelayout term for a container), but it should never be that either.

    
    

    
  
Yes, I did run layout regression tests and I couldn't find anything that
removing the hack would break.

    
    

    
  
Comment on attachment 87822 [details] [diff] [review]
Proposed fix

Well I did now.  It seems that <br />'s end up with a height of zero.

        Attachment #87822 -
        Flags: needs-work+

    
  
Keywords: review

    
    

    
  
nsbeta1-. Not a "stop ship" bug
Keywords: nsbeta1 → nsbeta1-

    
  
Severity: minor → normal

    
    

    
  
mass reassign to default owner
Assignee: karnaze → table
Status: ASSIGNED → NEW
QA Contact: amar → madhur
Target Milestone: Future → ---
Target Milestone: --- → Future

    
    

    
  
Seems to be fixed now...
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Keywords: qawanted

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: