Closed Bug 43882 Opened 21 years ago Closed 21 years ago

Blank line disappears during reflow.

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Platform:
x86
Windows NT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: kinmoz, Assigned: dbaron)

References

Details

(Whiteboard: [dogfood+][ETA: 6-29])

Attachments

(5 files)

Simple test case.
410 bytes, text/html
Details
The correct Simple Test Case. Sorry 'bout that.
440 bytes, text/html
Details
even simpler testcase
156 bytes, text/html
Details
another empty P quirk that still exists
243 bytes, text/html
Details
proposed patch
24.22 KB, patch
Details | Diff | Splinter Review 
In 4.x the following HTML renders a page with a blank line between the word 
"Test" and the table below it. In 6.x, it renders fine initially, but if you 
move the mouse over the button in the table, the blank line disappears and the 
table moves up against the word "Test".


<HTML><HEAD>
<TITLE>P-Table Bug</TITLE>
</HEAD>
<BODY>
Test
<p>
<FORM>
<TABLE BORDER=1>
  <TR>
    <TD><input type=button value=foo></TD>
    <TD><input value="kin@netscape.com"></TD>
  </TR>
</TABLE>
</FORM>
</BODY>
</HTML>


This bug is causing me a big headache in regards to Ender-Lite Textfields 
in tables because what happens sometimes during the loading of pages like 
bugzilla is that TextFields and their internal scrolledviews are layed out, with 
the blank line there, but then another reflow or incremental reflow comes along 
that gets rid of the blank line and shifts the table upwards ... but the 
scrolledviews in the textfields remain at their old positions because the table 
move caused an incremental reflow that for some reason doesn't touch the cells 
containing the TextFields.

This results in the TextField borders following the table, but the scrolledview 
which contains the textfield's contents stays where they were originally layed 
out at, so it looks like we are drawing outside of the textfield.

This problem becomes more of an issue with my fix for bug #42178 
which reduces the number of calls to FlushPendingNotifications() which are 
causing other problems.
Attached file Simple test case. — 
Adding karnaze@netscape.com to Cc list.
I have an older build and am seeing no space "Text" and the table initially. My 
guess is that the block incremental reflow code is to blame since it contains 
the <p> and the table.
Assignee: clayton → buster
Marking dependency - looks like this is blocking a dogfood+ nsbeta2+ bug.
Blocks: 42178
Just curious, who's picking up buster's bugs while he is on sabbatical till 
august?
The important ones (+) will get reassigned to waterson, dbaron, or me. If this 
one is important, please add the nsbeta2 keyword.
This seems similar to bug 43369.
I simplified the testcase a good bit more.  I think the problem is that there's 
something related to empty P elements that was not removed when buster removed 
the strange handling of empty P elements.  The problem is specific to P elements 
-- it cannot be seen with DIVs with margins.
Attached file even simpler testcase — 
This happens in both quirks mode and strict mode.
Assignee: buster → dbaron
There are still clearly P quirks in the 4 
files nsBlock{Frame,ReflowContext}.{h,cpp} .  I thought buster removed these 
(bug 35772 is marked VERIFIED-FIXED).  I'll try removing them tomorrow (I 
already have some half-baked changes in the files on this machine, and I'd 
rather not mess with patching them out) and see what happens.

Taking this bug in the hopes that that will fix it.
It looks like there's also stuff in nsCSSFrameConstructor.cpp .
Blocks: 43369
Adding dogfood keyword, since it blocks a dogfood+ bug. After talking to Kin, it 
sounds like there may be another (table) bug exposed with his fix to bug 42178. 
He will open a new bug and try to develop a test case without his fix; if that 
is not possible, then this test case can be used with his fix (which needs to be 
attached to bug 42178).
Keywords: dogfood
Putting on [dogfood+] radar, but only to unblock bug 42178.
Whiteboard: [dogfood+]
Blocks: 43991
Attached patch proposed patch — — Splinter Review 
Most of the changes in the patch above are indenting changes.  It still needs to 
be tested -- I need to run the block regression tests and see what I'm really 
changing :-)
I just tried dbaron's patch and it does prevent the blank line from 
disappearing, in the attatched test cases as well as bugzilla bug pages, so my 
Ender-Lite textfields and their scrolled views appear to be positioned 
correctly.

Having said that, I still think there is a table reflow bug ... if the table 
changes position, as it does when the blank line disappears, it needs to notify 
it's descendant frames so that they can sync their scrolled views with the new 
frame positions. In any case, that's a separate issue.
Whiteboard: [dogfood+] → [dogfood+][fix in hand(?)]
Any news on whether or not the fix is good? (Passes all regression tests, 
etc.)

Any ETA for checkin?
My fix actually causes the reverse problem in another case (that buster added
two weeks ago).  The regression tests aren't too useful here -- it's more my own
tests.  Hopefully I'll try to sort things out tonight.  This code isn't
tremendously easy to understand...
I'm going to try to check something in today.  I'm not sure which fix I'm going
to check in -- I want to discuss that with waterson, etc., but I intend to check
in something that will stop this bug.
Whiteboard: [dogfood+][fix in hand(?)] → [dogfood+][ETA: 6-29]
dbaron and I *just* talked about this. We think the right thing to do is to 
take out all of the empty paragraph special case code. This will expose a bug 
with collapsing borders, but that bug is fairly well understood. dbaron is 
going to troll through the regression tests one more time to make sure nothing 
else crops up and should be checking in today.
Status: NEW → ASSIGNED
Fix checked in.  This fix exposes existing bugs filed as bug 44242 and bug 
44264.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 43657 has been marked as a duplicate of this bug. ***
Fixed in the July 6th build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.