Closed Bug 53568 Opened 25 years ago Closed 25 years ago

HR's not given correct height, and height less than 2 causes very bad rendering on linux

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: jwbaker, Assigned: buster)

References

(
URL
)

Details

(Keywords: regression, testcase, top100, Whiteboard: [nsbeta3++])

Attachments

(6 files)

Minimal valid HTML 4.01 Transitional testcase
152 bytes, text/html
Details
frame dump with window at 2 different sizes
10.11 KB, text/plain
Details
Better testcase from bug 54261
468 bytes, text/html
Details
patch
1.04 KB, patch
Details | Diff | Splinter Review
screenshot of the <hr> bug @ CNN.com
109.12 KB, image/png
Details
Altavista, text-mode search... forms are obscured from the broken <hr>
45.31 KB, image/png
Details
Todays linux nightly (2000-09-21-06) has incorrect background colors and poor layout on many sites. http://www.theonion.com/ has grey column backgrounds which appear and disappear, and the words are not warpped around the images properly. http://cnn.com/ has grey backgrounds on half the page. http://bugzilla.mozilla.org/ has grey backgrounds in the form widgets where previoud builds have always had white backgrounds. Prefs colors are set correctly. The reproduce, simply visit any of the above URLs in this build.
Keywords: nsbeta3, top100
*** Bug 53625 has been marked as a duplicate of this bug. ***
stories w/comments at http://www.slashdot.org have looked terrible the past two builds. (now linux 2000092120)
usually clicking stop and then scroll the grey out of viewport and back cures the grey. But when one wants to view an animation... http://www.cnn.com/2000/TECH/space/09/21/hubble.stars/index.html (black-bug flavor)
same happens on http://mail.yahoo.com Very annoying.
hmm seems to me that animations have something to do with this: At cnn.com there's always an animated banner or two at top of page. The main page when loaded will show a big grey area below these. But if you scroll down (grey goes away) and then up again so far that you do NOT see the banners: the grey doesn't return. And: If you scroll further up, so the banners again are visible - the page will "refresh" to grey - possibly when the first cycle of animated frames is complete.
Buster: I am *guessing* that this is related to the HR stuff. Jerry: Could you create a minimised test case? (i.e. less than 1K) Thanks!
Assignee: clayton → buster
Keywords: qawanted
The attached testcase is only 152 bytes. The problem indeed lies with the HR element. And don't call me Jerry :)
Keywords: testcase
So is this a dup of 20582? It breaks a heap of sites - here a handful more http://www.microsoft.com/ http://www.nettavisen.no/ http://www.pcworld.no/ http://www3.nrk.no/
R.K.Aa- This is definitely not a dup of bug 20582 (though it may be related). The changes introduced to solve bug 20582 and bug 18754 were (AFAICT) put in on the 12th, and this bug did not occur until (IIRC) the 20th. Also, just for a further reference point/test case, this breaks http://espn.go.com.
Jeffrey: Dude, you rock, thanks!! :-D Buster, This is the <hr> problem of which there are a few dupes.
I'm not seeing this at all on my Windows NT build that includes all of the HR changes. So, either some other checkin tickled something to cause this, or it's linux only. All of you who have commented, please let me know if you were on a platform *other* than linux. Meanwhile, I'm pulling the latest to see what the heck is going on. If somebody with a debug build could do this, it would be *most* helpful: 1) run viewer with output redirected to a file. 2) in viewer, reproduce the problem with the tiny test case jwbaker attached (thanks!) 3) dump frames. 4) attach the frame dump to this bug.
Status: NEW → ASSIGNED
Blocks: 53908
= bug 53908
No longer blocks: 53908
*** Bug 53908 has been marked as a duplicate of this bug. ***
Did I mention that this bug is not manifest in strict HTML 4 documents?
*** Bug 53997 has been marked as a duplicate of this bug. ***
buster: this does appear to be Linux specific; I see this under the latest nightlies for Linux but can't duplicate it under W2K.
cc self. I _am_ using a linux build, BTW :)
*** Bug 54125 has been marked as a duplicate of this bug. ***
v1.19 of quirc.css was checked in by peierre for Ian Hickson on the 20th. Line 149 reads: -moz-box-sizing: border-box; changing this back to v.1.18 solves the problem: box-sizing: border-box;
errrr... make that quirk.css
*** Bug 54157 has been marked as a duplicate of this bug. ***
*** Bug 54168 has been marked as a duplicate of this bug. ***
adding regression keyw.
Keywords: regression
RK: backing that out would be treating the symptom, but not the cause. And it would cause other things to break. I've got a handle on this one. Expect a patch tomorrow. Thanks for your help!
*** Bug 54261 has been marked as a duplicate of this bug. ***
See bug 54261 for more commentary on the cause of this bug. HR's use -moz-bg-inset, and that causes borders to be added - but takes that space away from the content it's insetting. For HR size=2, the size of the content is 30, but the bottom and top borders are 15 each, leaving 0 for the content. For size=1, content gets -15. On Win32, the -15 content size causes a single line. On X11, the FillRect "escapes" because the rectangle is inside-out and really messes things up. However, all platforms are still rendering HR's (with and without noshade) incorrectly in all sizes due to this. (See my testcase.) Platform->All, OS->All, nominating for dogfood
Keywords: dogfood
OS: Linux → All
Hardware: PC → All
*** Bug 54383 has been marked as a duplicate of this bug. ***
*** Bug 54384 has been marked as a duplicate of this bug. ***
*** Bug 54412 has been marked as a duplicate of this bug. ***
adding mostfreq keyword
Keywords: mostfreq
Randell, R.K.Aa.: guys, you rock. Thanks for the help in tracking this down!
I have a very low risk fix in hand for this. Thanks to everyone who helped steer me in the right direction. Patch to be attached momentarily. PDT: I need a decision about whether this should be checked in for beta3 or rtm. The only change is to adjust the height of HRs, no crash or data corruption. But it does effect the visual display of many pages (some top100) on linux.
Priority: P3 → P1
Whiteboard: [fix in hand]
spam: that last comment was from me (buster), not marc. using his linux box to test...
changed subject to describe the cause of the problem
Summary: Poor layout, grey backgrounds everywhere → HR's not given correct height, and height less than 2 causes very bad rendering on linux
Attached patch patch — — Splinter Review
This sounds ugly... but does it really make the product unusable? (re: dogfood). I've put in an RTM nomination, and think that we'd want this given the small size of the patch. I put in an beta3- simply because it is too late to take this patch (we're trying to freeze today)
Keywords: rtm
Whiteboard: [fix in hand] → [nsbeta3-][fix in hand][need info]
I agree with Jar's assessment: not worth holding beta3 for. Some pages will display poorly on linux, but they will be usable.
chris and chris: can I get an 'r' and an 'a'?
I know my opinion doesn't matter a whole lot, but this bug appears on tons and tons of major websites. If you release a preview with this bug, you might as well not bother to compile the Linux binaries- you are going to get reamed in every single Linux publication for not being able to get something this simple right.You can go ahead and do it, but I for one will have to stop defending moz on /. and other places when they say "why can't mozilla render cnn correctly?"
This should be nsbeta3- only if you want to get raped by the unix press after the beta release. This bug is extremely visible and occurs on something like every web site that I visit, including major commercial sites, news sites, etc. Just to be clear for people who don't use Unix: this bug causes giant, ugly, flashing-when-scrolling grey boxes in the backgrounds of web pages. Releasing a beta with a layout bug of this visibility indicates that Mozilla can't perform even the most simple drawing operation. The rest of us know that isn't true, but we have to get simple things like <HR> right or nobody is going to take us seriously.
r=karnaze.
Adding rtm+.
Whiteboard: [nsbeta3-][fix in hand][need info] → [nsbeta3-][fix in hand][need info][rtm+]
Garrett, your first upload was scrambled due to Bug 52030, Bug 54081, and Bug 53341. But I think we get the point anyway.
Why not get the borders explicitly, and add them back? (Rather than assuming that they'll be 1px?) I presume that's what the 2px is for...
Just for the sake of adding my $0.02, if I downloaded a browser for the first time and as many pages started rendering with that gray crap as do with the current nightlies, I'd shut it down and go back to using what I'd been using previously. People do tend to judge things heavily based on outward appearance :)
OK... we have to spin again for tomorrow morn... and this is really bugging a lot of folks. I don't anticipate a "second chance" to improve this for PR3, but we'll take the fix that was proposed if it can land this afternoon (review, approved, etc). I've marked the bug beta3-double-plus to support the branch landing (Phil was also push for this transition)
Whiteboard: [nsbeta3-][fix in hand][need info][rtm+] → [nsbeta3++][fix in hand][need info][rtm+]
how the bug behaves at microsoft.com is even more interesting: images that are links vanish - all covered by the grey blocks. The only indication something is "there" is when cursor changes to a hand. In addition readability is so bad in some cases one has to select text in order to read it: contrast otherwise gets too low. I agree 110% with Jeffrey Baker, it must be fixed for nsbeta3. This bug is a disaster.
fix checked into branch. waiting for trunk to open to check in there as well.
Keywords: rtm
Whiteboard: [nsbeta3++][fix in hand][need info][rtm+] → [nsbeta3++][fix in hand]
why don't we fix it the "right way" on the tip?
*** Bug 54598 has been marked as a duplicate of this bug. ***
*** Bug 54607 has been marked as a duplicate of this bug. ***
*** Bug 54666 has been marked as a duplicate of this bug. ***
This problem is not manifest in latest Mozilla build (2000-09-28-21-M18). Is this fixed?
yep, fixed. checked into trunk last night.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Whiteboard: [nsbeta3++][fix in hand] → [nsbeta3++]
Fixed in the Sept 29 build.
Status: RESOLVED → VERIFIED
Doesn't look like this is fixed in the latest CVS build on Linux... http://quote.yahoo.com still shows a rather large grey box..
*** Bug 54610 has been marked as a duplicate of this bug. ***
Ari- I'm not seeing the problem on Debian with 200092909 on quote.yahoo.com. Let's not reopen this unless we are sure...
this is a pretty high-visibility bug, and I'd have expected to hear a lot of noisy if it wasn't really fixed. Ari, can you verify that you have the latest version of mozilla/layout/html/base/src/nsHRFrame.cpp, and that you've properly recompiled? If this is the case, contact me directly and we can figure out why you're seeing it. If anyone else is seeing the problem, please let me know asap.
I just pulled down a daily build and it looks fixed here. I don't see it on any of the pages that I was before.
Appears fixed on trunk FreeBSD 20001004xx I looked at the patch - it appears to compensate for -moz-bg-inset within the HR code. Is that the "correct" fix, or is -moz-bg-inset inherently incorrect? Also, it looked as if the code only compensated in quirks mode. Is that right? There's still a bug in rendering of noshade HR's with the endcaps, but that's another issue. I think someone got way too fancy with making the rounded endcaps.... I'm REALLY glad that it got fixed for the PR3 release - Not fixing it would have been asking to be pilloried. I'm also glad I spent the time to track it through the code.
Randell: I'm glad you pushed on this one, too! You are correct, I compensate for the inset computation inside the HR reflow code in quirks mode. That's what quirks mode is all about. This is to account for Nav's bizzare sizing algorithm with HR heights, and Nav/IE's odd treatment of HR's with respect to floaters. Basically, legacy behavior is to treat an HR as a block element unless it's impacted by a floater, in which case it's treated like an inline element. That's very tough for a CSS-compliant browser to emulate. This isn't necessary in standard mode at all, where HR's are block elements, period.
Apparently HR's aren't pure block elemented when printing.... See new bug 64632. I'm making this a new bug instead of reopening this one, though they're very closely related.
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: