We have some code in nsGenericHTMLElement::SetAttr to prevent processing of attribute changes that don't actually change the attribute. This code is bypassed if we call out to SetHTMLAttribute, however, and there is no equivalent check in SetHTMLAttribute. The URL field has an example page where our CPU usage is ridiculously higher than it should be because of this problem. In addition, nsGenericElement has no such checks at all; we probably want to add some. (same for nsSVGElement, and then there is XUL....) This bug depends on bug 188803; until that's checked in, fixing this bug would break inline style changes....

Comment on attachment 125802 [details] [diff] [review] Proposed patch (for generic XML and HTML only) Thoughts?

Comment on attachment 126086 [details] [diff] [review] Proposed patch including XUL and SVG - In nsXULElement::SetAttr(): if (tag == nsXULAtoms::window && aNodeInfo->Equals(nsXULAtoms::hidechrome)) { - nsAutoString val; - val.Assign(aValue); + nsAutoString val(aValue); HideWindowChrome(val.EqualsIgnoreCase("true")); How about using aValue.Equals("true", nsCaseInsensitiveStringComparator()) and loosing the nsAutoString? sr=jst

> How about using aValue.Equals("true", nsCaseInsensitiveStringComparator()) and > loosing the nsAutoString? That may give the wrong result in some locales, because the comparator is locale-sensitive.... See the problems we had with similar code when determining input types in HTML (bug number provided on request). Taking bug.

Comment on attachment 126086 [details] [diff] [review] Proposed patch including XUL and SVG >Index: content/svg/content/src/nsSVGAttributes.cpp >+ if (index < count) { // found the attr in the list >+ NS_ASSERTION(attr, "How did we get here with a null attr pointer?"); >+ modification = PR_TRUE; >+ attr->GetValue()->SetValueString(aValue); >+ } else { // didn't find it > if (GetMappedAttribute(aNodeInfo, &attr)) { > AppendElement(attr); > attr->GetValue()->SetValueString(aValue); > } > else { > rv = nsSVGAttribute::Create(aNodeInfo, aValue, &attr); > NS_ENSURE_TRUE(attr, rv); > AppendElement(attr); > } > attr->Release(); Add the comment we talked about, plus only one AppendElement() call. >Index: content/xul/content/src/nsXULElement.cpp >+ // Check to see if the STYLE attribute is being set. If so, we need to >+ // create a new style rule based off the value of this attribute, and we >+ // need to let the document know about the StyleRule change. > if (aNodeInfo->Equals(nsXULAtoms::style, kNameSpaceID_None) && >- (mDocument != nsnull)) { >+ mDocument) { This can fit on one line in under 80 chars now, if you like. I don't care one way or another. r=caillon.

fixed. This seems to not have affected Tp after all and improved Txul by 3%, Ts by 1% or so.

Nice work.

*** Bug 207054 has been marked as a duplicate of this bug. ***

The patch for nsXULElement.cpp caused the regression described in bug 212625 (verified by backing it out). Can anyone look at this, please?