Remove ClearUndisplayedContentMap hack from ReconstructStyleData (expanded for bug 118014; see bug 136728). Fix multiple content inserted notifications from XBL (the real cause of bug 118014) by checking the undisplayed content map in addition to the primary frame map. Fix FrameManager::GetUndisplayedContent to work correctly, and remove code in nsCSSFrameConstructor::AttributeChanged that was working around its brokenness. b=136704 sr=waterson r=attinasi

This commit is contained in:
dbaron%fas.harvard.edu 2002-05-01 00:36:50 +00:00
parent e4f5bd3810
commit eb7778a448
10 changed files with 111 additions and 109 deletions

View File

@ -52,7 +52,8 @@ REQUIRES = xpcom \
rdf \
imglib2 \
unicharutil \
webbrwsr \
webbrwsr \
locale \
$(NULL)
CPPSRCS = \

View File

@ -46,6 +46,7 @@ REQUIRES = xpcom \
rdf \
content_xul \
unicharutil \
locale \
$(NULL)
DEFINES=-D_IMPL_NS_HTML -DWIN32_LEAN_AND_MEAN

View File

@ -56,6 +56,8 @@
#include "nsNetUtil.h"
#include "nsXBLAtoms.h"
#include "nsINameSpaceManager.h"
#include "nsIFrameManager.h"
#include "nsIStyleContext.h"
NS_IMPL_ISUPPORTS1(nsXBLResourceLoader, nsICSSLoaderObserver)
@ -266,14 +268,31 @@ nsXBLResourceLoader::NotifyBoundElements()
if (parent)
parent->IndexOf(content, index);
// If |content| is (in addition to having binding |mBinding|)
// also a descendant of another element with binding |mBinding|,
// then we might have just constructed it due to the
// notification of its parent. (We can know about both if the
// binding loads were triggered from the DOM rather than frame
// construction.) So we have to check both whether the element
// has a primary frame and whether it's in the undisplayed map
// before sending a ContentInserted notification, or bad things
// will happen.
nsCOMPtr<nsIPresShell> shell;
doc->GetShellAt(0, getter_AddRefs(shell));
if (shell) {
nsIFrame* childFrame;
shell->GetPrimaryFrameFor(content, &childFrame);
nsCOMPtr<nsIDocumentObserver> obs(do_QueryInterface(shell));
if (!childFrame)
obs->ContentInserted(doc, parent, content, index);
if (!childFrame) {
// Check to see if it's in the undisplayed content map.
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
nsCOMPtr<nsIStyleContext> sc;
frameManager->GetUndisplayedContent(content, getter_AddRefs(sc));
if (!sc) {
nsCOMPtr<nsIDocumentObserver> obs(do_QueryInterface(shell));
obs->ContentInserted(doc, parent, content, index);
}
}
}
// Flush again

View File

@ -83,6 +83,8 @@
#include "nsIPresShell.h"
#include "nsIDocumentObserver.h"
#include "nsIFrameManager.h"
#include "nsIStyleContext.h"
#include "nsIXULPrototypeCache.h"
#include "nsIDOMLoadListener.h"
@ -150,14 +152,31 @@ public:
if (parent)
parent->IndexOf(mBoundElement, index);
// If |mBoundElement| is (in addition to having binding |mBinding|)
// also a descendant of another element with binding |mBinding|,
// then we might have just constructed it due to the
// notification of its parent. (We can know about both if the
// binding loads were triggered from the DOM rather than frame
// construction.) So we have to check both whether the element
// has a primary frame and whether it's in the undisplayed map
// before sending a ContentInserted notification, or bad things
// will happen.
nsCOMPtr<nsIPresShell> shell;
doc->GetShellAt(0, getter_AddRefs(shell));
if (shell) {
nsIFrame* childFrame;
shell->GetPrimaryFrameFor(mBoundElement, &childFrame);
nsCOMPtr<nsIDocumentObserver> obs(do_QueryInterface(shell));
if (!childFrame)
obs->ContentInserted(doc, parent, mBoundElement, index);
if (!childFrame) {
// Check to see if it's in the undisplayed content map.
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
nsCOMPtr<nsIStyleContext> sc;
frameManager->GetUndisplayedContent(mBoundElement, getter_AddRefs(sc));
if (!sc) {
nsCOMPtr<nsIDocumentObserver> obs(do_QueryInterface(shell));
obs->ContentInserted(doc, parent, mBoundElement, index);
}
}
}
}

View File

@ -10670,22 +10670,18 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
frameManager->GetUndisplayedContent(aContent, getter_AddRefs(styleContext));
#ifdef DEBUG
if (!styleContext) {
// Well, we don't have a context to use as a guide.
// Attempt #3 will be to resolve style if we at least have a parent frame.
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (parent) {
nsIFrame* parentFrame;
shell->GetPrimaryFrameFor(parent, &parentFrame);
if (parentFrame) {
nsCOMPtr<nsIStyleContext> parentContext;
parentFrame->GetStyleContext(getter_AddRefs(parentContext));
aPresContext->ResolveStyleContextFor(aContent, parentContext,
getter_AddRefs(styleContext));
}
NS_ASSERTION(!parentFrame,
"parent frame but no child frame or undisplayed entry");
}
}
#endif
}
}
}
@ -10719,22 +10715,19 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
frameManager->GetUndisplayedContent(aContent, getter_AddRefs(styleContext));
#ifdef DEBUG
if (!styleContext) {
// Well, we don't have a context to use as a guide.
// Attempt #3 will be to resolve style if we at least have a parent frame.
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (parent) {
nsIFrame* parentFrame;
shell->GetPrimaryFrameFor(parent, &parentFrame);
if (parentFrame) {
nsCOMPtr<nsIStyleContext> parentContext;
parentFrame->GetStyleContext(getter_AddRefs(parentContext));
aPresContext->ResolveStyleContextFor(aContent, parentContext,
getter_AddRefs(styleContext));
}
NS_ASSERTION(!parentFrame,
"parent frame but no child frame "
"or undisplayed entry");
}
}
#endif
}
//-----
}
@ -10759,6 +10752,8 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
!reframe) {
nsCOMPtr<nsIStyleSet> set;
shell->GetStyleSet(getter_AddRefs(set));
// XXXldb If |styleContext| is null, wouldn't it be faster to pass
// in something to tell it that this change is for inline style?
set->ClearStyleData(aPresContext, rule, styleContext);
}
@ -12178,6 +12173,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIPresContext* aPresContext,
// Now that the old frame is gone (and has stopped depending on obsolete style
// data), we need to blow away our style information if this reframe happened as
// a result of an inline style attribute changing.
// XXXldb Why does this look different from the code in
// |AttributeChanged|?
if (aInlineStyle) {
if (aStyleContext)
aStyleContext->ClearCachedDataForRule(aInlineStyleRule);

View File

@ -237,8 +237,6 @@ public:
nsresult RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode);
nsresult RemoveNodesFor(nsIContent* aParentContent);
nsresult GetNodeFor(nsIContent* aContent, nsIStyleContext** aResult);
// Removes all entries from the hash table
void Clear(void);
@ -828,13 +826,25 @@ FrameManager::ClearPlaceholderFrameMap()
NS_IMETHODIMP
FrameManager::GetUndisplayedContent(nsIContent* aContent, nsIStyleContext** aResult)
{
if (!aContent)
return NS_ERROR_NULL_POINTER;
NS_ENSURE_ARG_POINTER(aContent);
*aResult = nsnull; // initialize out param
if (mUndisplayedMap)
mUndisplayedMap->GetNodeFor(aContent, aResult);
if (!mUndisplayedMap)
return NS_OK;
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (!parent)
return NS_OK;
for (UndisplayedNode* node = mUndisplayedMap->GetFirstNode(parent);
node; node = node->mNext) {
if (node->mContent == aContent) {
*aResult = node->mStyle;
NS_ADDREF(*aResult);
return NS_OK;
}
}
return NS_OK;
}
@ -1731,8 +1741,6 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
}
else {
// XXXdwh figure this out.
// oldContext->RemapStyle(aPresContext, PR_FALSE);
if (pseudoTag && pseudoTag != nsHTMLAtoms::mozNonElementPseudo &&
aAttribute && (aMinChange < NS_STYLE_HINT_REFLOW) &&
HasAttributeContent(oldContext, aAttrNameSpaceID, aAttribute)) {
@ -1771,8 +1779,6 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
}
else {
// XXXdwh figure this out.
// oldExtraContext->RemapStyle(aPresContext, PR_FALSE);
// XXXldb |oldContext| is null by this point, so this will
// never do anything.
if (pseudoTag && pseudoTag != nsHTMLAtoms::mozNonElementPseudo &&
@ -1813,15 +1819,13 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
NS_IF_RELEASE(pseudoTag);
if (undisplayedContext) {
if (undisplayedContext == undisplayed->mStyle) {
// XXXdwh figure this out.
// undisplayedContext->RemapStyle(aPresContext);
}
const nsStyleDisplay* display =
(const nsStyleDisplay*)undisplayedContext->GetStyleData(eStyleStruct_Display);
if (display->mDisplay != NS_STYLE_DISPLAY_NONE) {
aChangeList.AppendChange(nsnull, ((undisplayed->mContent) ? undisplayed->mContent : localContent),
NS_STYLE_HINT_FRAMECHANGE);
// The node should be removed from the undisplayed map when
// we reframe it.
NS_RELEASE(undisplayedContext);
} else {
// update the undisplayed node with the new context
@ -2512,20 +2516,6 @@ UndisplayedMap::AddNodeFor(nsIContent* aParentContent, nsIStyleContext* aPseudoS
return AppendNodeFor(node, aParentContent);
}
nsresult
UndisplayedMap::GetNodeFor(nsIContent* aContent, nsIStyleContext** aResult)
{
PLHashEntry** entry = GetEntryFor(aContent);
if (*entry) {
UndisplayedNode* node = (UndisplayedNode*)((*entry)->value);
*aResult = node->mStyle;
NS_IF_ADDREF(*aResult);
}
else
*aResult = nsnull;
return NS_OK;
}
nsresult
UndisplayedMap::RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode)
{

View File

@ -5358,12 +5358,6 @@ PresShell::ReconstructStyleData(PRBool aRebuildRuleTree)
set->BeginRuleTreeReconstruct();
}
// Clear all undisplayed content in the undisplayed content map.
// These cached style contexts will no longer be valid
// HACK? see bug 118014 - this is really be done because some XUL anonymous content
// may have been removed but not cleared from the undisplayed map
frameManager->ClearUndisplayedContentMap();
PRInt32 frameChange = NS_STYLE_HINT_NONE;
frameManager->ComputeStyleChangeFor(mPresContext, rootFrame,
kNameSpaceID_Unknown, nsnull,

View File

@ -237,8 +237,6 @@ public:
nsresult RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode);
nsresult RemoveNodesFor(nsIContent* aParentContent);
nsresult GetNodeFor(nsIContent* aContent, nsIStyleContext** aResult);
// Removes all entries from the hash table
void Clear(void);
@ -828,13 +826,25 @@ FrameManager::ClearPlaceholderFrameMap()
NS_IMETHODIMP
FrameManager::GetUndisplayedContent(nsIContent* aContent, nsIStyleContext** aResult)
{
if (!aContent)
return NS_ERROR_NULL_POINTER;
NS_ENSURE_ARG_POINTER(aContent);
*aResult = nsnull; // initialize out param
if (mUndisplayedMap)
mUndisplayedMap->GetNodeFor(aContent, aResult);
if (!mUndisplayedMap)
return NS_OK;
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (!parent)
return NS_OK;
for (UndisplayedNode* node = mUndisplayedMap->GetFirstNode(parent);
node; node = node->mNext) {
if (node->mContent == aContent) {
*aResult = node->mStyle;
NS_ADDREF(*aResult);
return NS_OK;
}
}
return NS_OK;
}
@ -1731,8 +1741,6 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
}
else {
// XXXdwh figure this out.
// oldContext->RemapStyle(aPresContext, PR_FALSE);
if (pseudoTag && pseudoTag != nsHTMLAtoms::mozNonElementPseudo &&
aAttribute && (aMinChange < NS_STYLE_HINT_REFLOW) &&
HasAttributeContent(oldContext, aAttrNameSpaceID, aAttribute)) {
@ -1771,8 +1779,6 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
}
else {
// XXXdwh figure this out.
// oldExtraContext->RemapStyle(aPresContext, PR_FALSE);
// XXXldb |oldContext| is null by this point, so this will
// never do anything.
if (pseudoTag && pseudoTag != nsHTMLAtoms::mozNonElementPseudo &&
@ -1813,15 +1819,13 @@ FrameManager::ReResolveStyleContext(nsIPresContext* aPresContext,
}
NS_IF_RELEASE(pseudoTag);
if (undisplayedContext) {
if (undisplayedContext == undisplayed->mStyle) {
// XXXdwh figure this out.
// undisplayedContext->RemapStyle(aPresContext);
}
const nsStyleDisplay* display =
(const nsStyleDisplay*)undisplayedContext->GetStyleData(eStyleStruct_Display);
if (display->mDisplay != NS_STYLE_DISPLAY_NONE) {
aChangeList.AppendChange(nsnull, ((undisplayed->mContent) ? undisplayed->mContent : localContent),
NS_STYLE_HINT_FRAMECHANGE);
// The node should be removed from the undisplayed map when
// we reframe it.
NS_RELEASE(undisplayedContext);
} else {
// update the undisplayed node with the new context
@ -2512,20 +2516,6 @@ UndisplayedMap::AddNodeFor(nsIContent* aParentContent, nsIStyleContext* aPseudoS
return AppendNodeFor(node, aParentContent);
}
nsresult
UndisplayedMap::GetNodeFor(nsIContent* aContent, nsIStyleContext** aResult)
{
PLHashEntry** entry = GetEntryFor(aContent);
if (*entry) {
UndisplayedNode* node = (UndisplayedNode*)((*entry)->value);
*aResult = node->mStyle;
NS_IF_ADDREF(*aResult);
}
else
*aResult = nsnull;
return NS_OK;
}
nsresult
UndisplayedMap::RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode)
{

View File

@ -5358,12 +5358,6 @@ PresShell::ReconstructStyleData(PRBool aRebuildRuleTree)
set->BeginRuleTreeReconstruct();
}
// Clear all undisplayed content in the undisplayed content map.
// These cached style contexts will no longer be valid
// HACK? see bug 118014 - this is really be done because some XUL anonymous content
// may have been removed but not cleared from the undisplayed map
frameManager->ClearUndisplayedContentMap();
PRInt32 frameChange = NS_STYLE_HINT_NONE;
frameManager->ComputeStyleChangeFor(mPresContext, rootFrame,
kNameSpaceID_Unknown, nsnull,

View File

@ -10670,22 +10670,18 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
frameManager->GetUndisplayedContent(aContent, getter_AddRefs(styleContext));
#ifdef DEBUG
if (!styleContext) {
// Well, we don't have a context to use as a guide.
// Attempt #3 will be to resolve style if we at least have a parent frame.
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (parent) {
nsIFrame* parentFrame;
shell->GetPrimaryFrameFor(parent, &parentFrame);
if (parentFrame) {
nsCOMPtr<nsIStyleContext> parentContext;
parentFrame->GetStyleContext(getter_AddRefs(parentContext));
aPresContext->ResolveStyleContextFor(aContent, parentContext,
getter_AddRefs(styleContext));
}
NS_ASSERTION(!parentFrame,
"parent frame but no child frame or undisplayed entry");
}
}
#endif
}
}
}
@ -10719,22 +10715,19 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
nsCOMPtr<nsIFrameManager> frameManager;
shell->GetFrameManager(getter_AddRefs(frameManager));
frameManager->GetUndisplayedContent(aContent, getter_AddRefs(styleContext));
#ifdef DEBUG
if (!styleContext) {
// Well, we don't have a context to use as a guide.
// Attempt #3 will be to resolve style if we at least have a parent frame.
nsCOMPtr<nsIContent> parent;
aContent->GetParent(*getter_AddRefs(parent));
if (parent) {
nsIFrame* parentFrame;
shell->GetPrimaryFrameFor(parent, &parentFrame);
if (parentFrame) {
nsCOMPtr<nsIStyleContext> parentContext;
parentFrame->GetStyleContext(getter_AddRefs(parentContext));
aPresContext->ResolveStyleContextFor(aContent, parentContext,
getter_AddRefs(styleContext));
}
NS_ASSERTION(!parentFrame,
"parent frame but no child frame "
"or undisplayed entry");
}
}
#endif
}
//-----
}
@ -10759,6 +10752,8 @@ nsCSSFrameConstructor::AttributeChanged(nsIPresContext* aPresContext,
!reframe) {
nsCOMPtr<nsIStyleSet> set;
shell->GetStyleSet(getter_AddRefs(set));
// XXXldb If |styleContext| is null, wouldn't it be faster to pass
// in something to tell it that this change is for inline style?
set->ClearStyleData(aPresContext, rule, styleContext);
}
@ -12178,6 +12173,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIPresContext* aPresContext,
// Now that the old frame is gone (and has stopped depending on obsolete style
// data), we need to blow away our style information if this reframe happened as
// a result of an inline style attribute changing.
// XXXldb Why does this look different from the code in
// |AttributeChanged|?
if (aInlineStyle) {
if (aStyleContext)
aStyleContext->ClearCachedDataForRule(aInlineStyleRule);