Closed
Bug 341062
Opened 18 years ago
Closed 18 years ago
[FIX]regression from seamonkey 1.1a, page rendered incorrectly
Categories
(Core :: DOM: HTML Parser, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: rambousek, Assigned: bzbarsky)
References
()
Details
(Keywords: testcase)
Attachments
(3 files, 2 obsolete files)
23.82 KB,
text/plain
|
Details | |
295 bytes,
text/html
|
Details | |
1.69 KB,
patch
|
mrbkap
:
review+
sicking
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a1) Gecko/20060610 SeaMonkey/1.5a Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a1) Gecko/20060610 SeaMonkey/1.5a In Gecko/20060605 SeaMonkey/1.1a, this page is rendered alright, but in any build of SeaMonkey/1.5a the content of large frame is somehow doubled and it's not possible to fill in the form fields. The page uses lot of Javascript, so it's possible the bug is somehow related to it. Reproducible: Always
Comment 1•18 years ago
|
||
The change on the trunk was due to 332644 (verified via backout), but the page doesn't load properly if I load it locally, so I don't know what's actually going on.
Assignee: general → mrbkap
Component: General → HTML: Parser
Product: Mozilla Application Suite → Core
QA Contact: general → parser
Version: unspecified → Trunk
Assignee | ||
Comment 2•18 years ago
|
||
Hmm... I'd have thought this would be a duplicate of bug 339249, but I seem to see this in builds that postdate that fix. I'll look into this, but if someone can come up with a minimal testcase that would be wonderful....
Assignee | ||
Comment 3•18 years ago
|
||
The problem seems to be that we have two HTMLContentSinks around that have the same mDocument: (gdb) p this $23 = (HTMLContentSink * const) 0x8f917d0 (gdb) p ((nsParser*)((nsHTMLDocument*)mDocument.mRawPtr)->mParser.mRawPtr)->mSink.mRawPtr $24 = (nsIContentSink *) 0x8f9181c (gdb) p ((class HTMLContentSink*)((nsParser*)((nsHTMLDocument*)mDocument.mRawPtr)->mParser.mRawPtr)->mSink.mRawPtr)->mDocument $26 = {mRawPtr = 0x91b8630} (gdb) p mDocument $27 = {mRawPtr = 0x91b8630} We end up doing an append notification on |this|, which triggers editor munging the content model, which flushes tags on the other sink. Since the mInNotification member is per-sink, the other sink doesn't know we're in the middle of a flush ourselves... Blake, any idea what's going on here? document.write is _definitely_ involved and the CloseBody() call comes from document.close() being called in JS.
Comment 4•18 years ago
|
||
This took a while to reduce :).
Comment 5•18 years ago
|
||
This still shows the problem on the initial load, but doesn't happen again if you [shift+]reload.
Attachment #225211 -
Attachment is obsolete: true
Assignee | ||
Comment 6•18 years ago
|
||
In brief, the terminated parser's sink didn't die right away, so kept observing the document. Since the <html> node is not changed when we do document.open(), both sinks had contexts pointing to it; the old sink suddenly saw new content on that context when the new head and body got created, and decided it had to notify. The patch just stops observing the document at what looks like the right spot to me in the sink.
Attachment #225218 -
Flags: superreview?(mrbkap)
Attachment #225218 -
Flags: review?(mrbkap)
Assignee | ||
Updated•18 years ago
|
Attachment #225218 -
Flags: superreview?(mrbkap) → superreview?(bugmail)
Assignee | ||
Updated•18 years ago
|
Assignee: mrbkap → bzbarsky
Priority: -- → P1
Summary: regression from seamonkey 1.1a, page rendered incorrectly → [FIX]regression from seamonkey 1.1a, page rendered incorrectly
Target Milestone: --- → mozilla1.9alpha
Comment 7•18 years ago
|
||
Comment on attachment 225218 [details] [diff] [review] Proposed fix This patch seems fine on its own, but I think you're missing nsHTMLContentSink.cpp (i.e., the actual bug fix)...
Did you attach the wrong patch here?
Assignee | ||
Comment 9•18 years ago
|
||
Er... yes. This is the real fix. It's just one line of code and some comments.
Attachment #225218 -
Attachment is obsolete: true
Attachment #225349 -
Flags: superreview?(bugmail)
Attachment #225349 -
Flags: review?(mrbkap)
Attachment #225218 -
Flags: superreview?(bugmail)
Attachment #225218 -
Flags: review?(mrbkap)
Comment 10•18 years ago
|
||
Comment on attachment 225349 [details] [diff] [review] Actual patch Did you figure out what was keeping the content sink alive? That might be worth fixing as well (if possible).
Attachment #225349 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 11•18 years ago
|
||
> Did you figure out what was keeping the content sink alive?
Probably the fact that the parser is alive. This is kept alive by the termination function set by the first document.close() call. I'm not sure that's something we want to "fix".
Attachment #225349 -
Flags: superreview?(bugmail) → superreview+
Assignee | ||
Comment 12•18 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•