bugzilla "show_bug.cgi" form repeats data

VERIFIED FIXED

Status

()

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

People

(Reporter: Chris Waterson, Assigned: Chris Waterson)

Tracking

({regression, smoketest})

Trunk
regression, smoketest
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

18 years ago
Visit an existing bug. Any bug. You'll see the bugzilla form data repeated, at
least once, maybe more times. You'll crash when you leave the page.
(Assignee)

Updated

18 years ago
Keywords: dogfood, nsbeta2, regression
(Assignee)

Comment 1

18 years ago
Created attachment 11447 [details]
minimized test case
(Assignee)

Comment 2

18 years ago
Created attachment 11448 [details]
minimized even more
(Assignee)

Updated

18 years ago
Assignee: asa → waterson
Component: Browser-General → Layout
Summary: bugzilla "show_bug.cgi" form repeats data → {ib?} bugzilla "show_bug.cgi" form repeats data
(Assignee)

Comment 3

18 years ago
This looks like it could be a parser bug compounded with an {ib} bug. The first 
test case produces what appears to be an incorrect content model due to content 
rotation by the parser. The second test case appears to produce the classic 
{ib} problem: the content model is correct but the frame model is horked. 
cc'ing nisheeth and harishd.

I'll start by debugging the {ib} problem in the second test case; harishd, 
could you take a look at the content model produced by the first test case?
(Assignee)

Comment 4

18 years ago
Ok, I did some more experimenting. This is definitely *not* an {ib} bug, 
because we're never hitting the inline-block construction code. Turns out 
that a <form> is "display: block", so I was just confused.

Here's what I've found out. With this content:

 <FORM><input type="radio"></input><input type="radio"></input><p></p></FORM>

(note the closed <p></p> tag pair), I generate the following frame tree:

 Block(body)(1)@00D53C90 {120,120,8940,270} [state=00000004]<
   line 00D53EF4: count=1 <
     Block(form)(0)@00D53D00 {0,0,8940,270} [state=00000004] <
       line 00D53EA4: count=2 <
         Frame(input)(0)@00D53D64 {45,30,180,180} [state=00000024]
         Frame(input)(1)@00D53DE0 {345,30,180,180} [state=00000024]
       >
       line 00D53ECC: count=1 <
         Block(p)(2)@00D53E5C {0,270,8940,0} [state=00000004] <
         >
       >
     >
   >
 >

With this content:

(note the unclosed <p> tag), I generate the following frame tree:


 Block(body)(1)@00D53C90 {120,120,8940,510} [state=00000014] <
   line 00D53EF4: count=1 <
     Block(form)(0)@00D53D00 next=00D53D64 {0,0,8940,0} [state=00000004] <
     >
   >
   line 00D53F90: count=3 {0,240,900,270} <
     Frame(input)(1)@00D53D64 {45,270,180,180} [state=00000024]
     Frame(input)(1)@00D53DE0 {345,270,180,180} [state=00000024]
     Frame(input)(2)@00D53FB8 {645,270,180,180} [state=00000024]
   >
   line 00D53ECC: count=1 <
     Block(p)(3)@00D53E5C {0,510,8940,0} [state=00000004] <
     >
   >
 >

Note how the input frames have been promoted up outside the <form>! I've been 
setting breakpoints in nsCSSFrameConstructor::ContentAppended() and 
ContentRemoved(), and they're being called several times from the content 
sink's rotation code.

I'll next verify that they are sending correct information to layout. My 
suspicion is that the <p>-closing code is interacting poorly with the 
form-element promotion code, and is not sending layout enough information to 
properly destroy and reconstruct frames.

adding vidur & jst to cc list.
Summary: {ib?} bugzilla "show_bug.cgi" form repeats data → bugzilla "show_bug.cgi" form repeats data

Comment 5

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

Comment 6

18 years ago
adding the smoketest keyword to ensure that we don't open the tree without 
getting this fixed by monday. 
Keywords: smoketest
(Assignee)

Comment 7

18 years ago
Ok, this was caused by dbaron's recent checkin to nsHTMLInputElement.cpp. What 
happens is something like this.

- The HTML content sink is in the midst of DemoteContainer(), which is removing 
elements out of the <form> and promoting them to the <body>. In the process of 
doing so, it will call nsIContent::SetDocument().

- When it calls this on an nsHTMLInputElement, the SetDocument() implementation 
makes a call to GetPrimaryFrame(). (It does this to eventually get the 
nsIEditor object, which is entraining a pile of anonymous content. This is the 
new code that dbaron added.)

- GetPrimaryFrame() by default will call 
nsHTMLDocument::FlushPendingNotifications() unless you call 
it with aFlushNotifcations set to PR_FALSE.

- FlushPendingNotifications() re-enters the content sink at this inopportune 
time (while it is doing promotion) and generally sets all hell loose.

Calling GetPrimaryFrame() with aFlushNotifcations == PR_FALSE fixes this. There 
is no reason to flush notifications at this point.
(Assignee)

Comment 8

18 years ago
Created attachment 11451 [details] [diff] [review]
fix
(Assignee)

Comment 9

18 years ago
fix checked in, r=mozbot
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Thanks for fixing this, Chris.  I didn't think GetPrimaryFrame would do anything
other than get the primary frame.  But the main problem was that I didn't test
this change well enough.

Comment 11

18 years ago
Sure this fix is 100%?

There's one plagued little page out there that now has an even harder time than
before:
http://www.locigames.com

Now also renders twice :)

Comment 12

18 years ago
R.K.Aa, i think the lokigames problem is the result of strict-dtd, but I'm not
sure.  I mentioned it on the dtd collection bug.

Comment 13

18 years ago
i visit lokigames regularly these days for news about SimCity. When this bug got
active was the first time i saw it render twice..

Comment 14

18 years ago
bug 45614 is another new one, image at yahoo.com loads twice.

Comment 15

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

Comment 16

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

Comment 17

18 years ago
Sorry - bug 45644 is NOT a dup.

Comment 18

18 years ago
I'm still seeing this randomly on some bug reports, 081909. If I see it again
(since it doesn't happen every time), I'll attach a screen shot. Reopen, anyone?

Comment 19

18 years ago
Created attachment 13322 [details]
Still occurs - 082108, win32 (despite what title bar build id is)
setting default qa
QA Contact: doronr → petersen

Comment 21

18 years ago
Not able to reproduce problem on the Sept 5th build.Tested on Win 98 and Mac OS 
9. Marking verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.