Blank line disappears during reflow.

VERIFIED FIXED

Status

()

Core
Layout
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: kinmoz, Assigned: dbaron)

Tracking

Trunk
x86
Windows NT
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(5 attachments)

(Reporter)

Description

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

Comment 1

18 years ago
Created attachment 10673 [details]
Simple test case.
(Reporter)

Comment 2

18 years ago
Adding karnaze@netscape.com to Cc list.
(Reporter)

Comment 3

18 years ago
Created attachment 10674 [details]
The correct Simple Test Case. Sorry 'bout that.

Comment 4

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

Comment 5

18 years ago
Marking dependency - looks like this is blocking a dogfood+ nsbeta2+ bug.
Blocks: 42178
(Reporter)

Comment 6

18 years ago
Just curious, who's picking up buster's bugs while he is on sabbatical till 
august?

Comment 7

18 years ago
The important ones (+) will get reassigned to waterson, dbaron, or me. If this 
one is important, please add the nsbeta2 keyword.
(Assignee)

Comment 8

18 years ago
This seems similar to bug 43369.
(Assignee)

Comment 9

18 years ago
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.
(Assignee)

Comment 10

18 years ago
Created attachment 10686 [details]
even simpler testcase
(Assignee)

Comment 11

18 years ago
This happens in both quirks mode and strict mode.
(Assignee)

Comment 12

18 years ago
Created attachment 10688 [details]
another empty P quirk that still exists
(Assignee)

Updated

18 years ago
Assignee: buster → dbaron
(Assignee)

Comment 13

18 years ago
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.
(Assignee)

Comment 14

18 years ago
It looks like there's also stuff in nsCSSFrameConstructor.cpp .
(Assignee)

Updated

18 years ago
Blocks: 43369

Comment 15

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

Comment 16

18 years ago
Putting on [dogfood+] radar, but only to unblock bug 42178.
Whiteboard: [dogfood+]

Updated

18 years ago
Blocks: 43991
(Assignee)

Comment 17

18 years ago
Created attachment 10723 [details] [diff] [review]
proposed patch
(Assignee)

Comment 18

18 years ago
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 :-)
(Reporter)

Comment 19

18 years ago
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.
(Assignee)

Updated

18 years ago
Whiteboard: [dogfood+] → [dogfood+][fix in hand(?)]
(Reporter)

Comment 20

18 years ago
Any news on whether or not the fix is good? (Passes all regression tests, 
etc.)

Any ETA for checkin?
(Assignee)

Comment 21

18 years ago
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...
(Assignee)

Comment 22

18 years ago
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]

Comment 23

18 years ago
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.
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 24

18 years ago
Fix checked in.  This fix exposes existing bugs filed as bug 44242 and bug 
44264.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

18 years ago
*** Bug 43657 has been marked as a duplicate of this bug. ***

Comment 26

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