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

RESOLVED FIXED in Future

Status

()

Core
Layout: Tables
P2
normal
RESOLVED FIXED
16 years ago
3 years ago

People

(Reporter: Aaron Kaluszka, Unassigned)

Tracking

({regression, testcase})

Trunk
Future
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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.
(Reporter)

Updated

16 years ago
Keywords: 4xp, regression
Does removing the doctype declaration from the document help?  This may well be 
a standards mode only issue.
(Reporter)

Comment 2

16 years ago
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

Comment 4

16 years ago
Created attachment 62762 [details]
testcase

Comment 5

16 years ago
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.

Comment 6

16 years ago
Created attachment 62763 [details]
corrected testcase
Attachment #62762 - Attachment is obsolete: true
(Reporter)

Comment 7

16 years ago
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.

Comment 8

16 years ago
Created attachment 63103 [details]
Testcase #3

Seems like the table cell counts the <BR> as 1px wide?

Updated

16 years ago
Severity: normal → minor
Keywords: testcase

Comment 9

16 years ago
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

Comment 10

16 years ago
Temporarily moving to future until a milestone can be assigned. 
Status: NEW → ASSIGNED
Target Milestone: --- → Future
(Reporter)

Updated

16 years ago
Keywords: mozilla0.9.9

Comment 11

16 years ago
Not platform specific (seeing on Linux too).
OS: Windows XP → All
Hardware: PC → All
(Reporter)

Comment 12

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

Comment 13

16 years ago
the fix for bug 10036 was a hack, and now we pay the price for this.
(Reporter)

Comment 14

16 years ago
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.
(Reporter)

Updated

16 years ago
Keywords: mozilla0.9.9 → mozilla1.0, nsbeta1
nsbeta1-. Engineers are overloaded with higher priority bugs.
Keywords: nsbeta1 → nsbeta1-
(Reporter)

Comment 16

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

Updated

16 years ago
Priority: -- → P2
(Reporter)

Comment 17

16 years ago
Too bad display:block severely breaks NS4.x
(Reporter)

Updated

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

Comment 18

16 years ago
Created attachment 87822 [details] [diff] [review]
Proposed fix

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

Updated

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

Comment 19

16 years ago
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.
(Reporter)

Comment 22

16 years ago
Yes, I did run layout regression tests and I couldn't find anything that
removing the hack would break.
(Reporter)

Comment 23

16 years ago
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+
(Reporter)

Updated

16 years ago
Keywords: review
nsbeta1-. Not a "stop ship" bug
Keywords: nsbeta1 → nsbeta1-
(Reporter)

Updated

15 years ago
Severity: minor → normal

Comment 25

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

Updated

15 years ago
Target Milestone: --- → Future
(Reporter)

Comment 26

15 years ago
Seems to be fixed now...
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.