Closed
Bug 332644
Opened 19 years ago
Closed 19 years ago
[FIX]Possible to double-notify on <head> element in HTMLContentSink
Categories
(Core :: DOM: HTML Parser, defect, P2)
Core
DOM: HTML Parser
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file, 2 obsolete files)
|
6.49 KB,
patch
|
mrbkap
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
Due to the way the notifications happen, esp. the "start at 1 on the stack" mess, we can end up notifying twice on the <head>. That doesn't assert right then, but asserts if the <head> is ever removed from the document...
I'm going to add an assert to catch the double-notify earlier, and a fix for the actual problem.
|
Assignee | |
Comment 1•19 years ago
|
||
I got dbaron's OK on the nsFrameManager change, so it's just the content sink changes that need reviewing.
I doubt we want to risk this on branches. :(
Attachment #217118 -
Flags: superreview?(peterv)
Attachment #217118 -
Flags: review?(mrbkap)
|
|
Assignee | |
Updated•19 years ago
|
Priority: -- → P2
|
||
Comment 2•19 years ago
|
||
Comment on attachment 217118 [details] [diff] [review]
Fix
>Index: content/html/document/src/nsHTMLContentSink.cpp
>===================================================================
>+ // Note that we can start at stackPos == 0 here, because it's the caller's
>+ // responsibility to handle flushing interactions between contexts (see
>+ // HTMLContentSink::BeginContext).
> PRInt32 stackPos = 1;
Did you mean == 1 in the comment?
>@@ -1790,19 +1794,20 @@ SinkContext::FlushTags(PRBool aNotify)
> }
>
> void
> SinkContext::UpdateChildCounts()
> {
> // Start from the top of the stack (growing upwards) and see if any
'growing downward'?
Attachment #217118 -
Flags: superreview?(peterv) → superreview+
|
||
Comment 3•19 years ago
|
||
Comment on attachment 217118 [details] [diff] [review]
Fix
r=mrbkap with the comment changes suggested by peterv.
Attachment #217118 -
Flags: review?(mrbkap) → review+
|
|
Assignee | |
Comment 4•19 years ago
|
||
> Did you mean == 1 in the comment?
No, I meant to change the code to use 0 instead of 1. Then I was testing some more, and clearly forgot to change it back to 0. :(
I guess this needs review again, with that change. :(
Attachment #217118 -
Attachment is obsolete: true
Attachment #217437 -
Flags: superreview?(peterv)
Attachment #217437 -
Flags: review?(mrbkap)
|
|
||
Updated•19 years ago
|
Attachment #217437 -
Flags: review?(mrbkap) → review+
|
|
Assignee | |
Comment 5•19 years ago
|
||
Comment on attachment 217437 [details] [diff] [review]
Fixed issues
With bug 310985 checked in, this patch catches a bug in our notifications for head and body.... I'll update the patch.
Attachment #217437 -
Flags: superreview?(peterv) → superreview-
|
|
Assignee | |
Comment 6•19 years ago
|
||
Attachment #217437 -
Attachment is obsolete: true
Attachment #218855 -
Flags: superreview?(peterv)
Attachment #218855 -
Flags: review?(mrbkap)
|
|
||
Updated•19 years ago
|
Attachment #218855 -
Flags: superreview?(peterv) → superreview+
|
|
||
Updated•19 years ago
|
Attachment #218855 -
Flags: review?(mrbkap) → review+
|
|
Assignee | |
Comment 7•19 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•