Closed Bug 21225 Opened 25 years ago Closed 24 years ago

Changing element's 'class' causes unnecessary reflow

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: mikepinkerton, Assigned: attinasi)

References

()

Details

Attachments

(1 file)

Go to www.fool.com
mouse over any of the green bg table cells (like 'Stock Strategies' or 'News &
Commentary')

notice on the console the table/block code spits out its reflow warnings again
every time you move the mouse w/in this cell.

assigning to someone on the repaint team. can't test in viewer, mac viewer can't
load external urls.
Status: NEW → ASSIGNED
Target Milestone: M13
It reflows in viewer on WIN32 as well.
Each of the headings has a onmouseover handler which sets the class attribute.

<td class="green" onmouseover="this.className='orange'"
onmouseout="this.className='green'" onclick="navgo('/news.htm');">&nbsp;<A
class="white" href="/news.htm"><FONT face=Arial, Helvetica color=#f0f0df
size=+1>News &amp; Commentary</font></A></TD>

On IE5 setting the class attribute causes the a style sheet to become active
which sets the background of table cell from green to blue. On Navigator 4.7 it
does nothing. On Mozilla it does not change the color but causes a resize flow.
The reflow goes from incremental to resize in nsBlockReflowContext::ReflowBlock.

We do need to reflow, but since it's only a color change it should not be
changed into a resize reflow. The other issue is that it isn't changing color.
Need to make sure page is using standard DOM/CSS.
Simplier test case which generates resize reflows when mousing over the table
cell.

<HTML>
<STYLE>
.orange {BACKGROUND: red; }
.green { BACKGROUND: green; }
</STYLE>
<BODY>
<TABLE width="96%" cellpadding=2 cellspacing=2 border=0>
<TR>
<td class="green" onmouseover="this.className='orange'"
onmouseout="this.className='green'"><p>Retirement</p></TD>
</TR>
</TABLE>
</BODY>
</HTML>
Assignee: kmcclusk → troy
Status: ASSIGNED → NEW
My guess is the  nsBlockReflowContext::ReflowBlock logic which converts
the incremental reflows generated on the mouseover to resize reflows is
incorrect.

Troy - Chris Karnaze said you were going to be changing the block reflow logic
so I'm reassiging to you.
Assignee: troy → pierre
Status: ASSIGNED → NEW
Component: Layout → Style System
I don't see why this bug is assigned to me. Pinkerton is complaining that we're
doing a reflow. That would be the style system that is causing the
[presumably] style changed reflow
Block-moved to attinasi the bugs related to style inside tables
Assignee: pierre → attinasi
Status: NEW → ASSIGNED
The AttributeChanged call on the nsCSSFrameConstructor calls out to the 
StyledContent interface on the content node, which then calls  
nsGenericHTMLElement::GetCommonMappedAttributesImpact which says it wants a 
reframe when the attribute changed is a class:

PRBool
nsGenericHTMLElement::GetCommonMappedAttributesImpact(const nsIAtom* aAttribute,
                                                      PRInt32& aHint)
{
  if (nsHTMLAtoms::dir == aAttribute) {
    aHint = NS_STYLE_HINT_REFLOW;  // XXX really? possibly FRAMECHANGE?
    return PR_TRUE;
  }
  else if (nsHTMLAtoms::kClass == aAttribute) {		// bug 8862
    aHint = NS_STYLE_HINT_FRAMECHANGE;
    return PR_TRUE;
  }
  else if (nsHTMLAtoms::_baseHref == aAttribute) {
    aHint = NS_STYLE_HINT_VISUAL; // at a minimum, elements may need to override
    return PR_TRUE;
  }
  return PR_FALSE;
}

As noted in the comment above, bug 8862 was the cause of this action on class 
change. That bug indicates that a class change should cause a reframe. It seems 
to me that changing the class should not necessarily cause a reframe (consider 
the case where the old and new class have the same style!) but should allow the 
usual style impact determination to be done. 

Removing the NS_STYLE_HINT_FRAMECHANGE for a class change from the method above 
makes the reflow go away. Also, the problem reported in Bug 8862 is not a 
problem with the change removed, meaning it works as it should when the class 
change does not cause a reframe (some other change fixed it?)

The fix for this is to remove the code pierre added to nsGenericHTMLElement but 
I want to check with Pierre first...
You can't remove the fix for bug 8862 in 
nsGenericHTMLElement::GetCommonMappedAttributesImpact() otherwise the testcase 
shows the problem again. If you want to try it, the testcase is now on
http://members.aol.com/supersamat/dhtml/dom/toc/table-of-contents.htm

To reproduce: 
- click on Directory 1 (click on the text, not on the icon) 
==> you see that the icon changes from a closed folder to an open folder 
- click on Directory 2 
==> BUG: the icon doesn't change 
- cliick on Directory 3 
==> BUG: the icon doesn't change either 
- resize the window 
==> the icons are correctly updated
It looks like the REFRAME that we are doing as the result of a class change is 
really masking another bug in BlockFrame where changing a list item's image does 
not cause the correct area to be invalidated. I'll talk to buster about it and 
see what he thinks about it. Leaving the REFRAME in in the meantime.
The real bug was in nsBulletFrame in that it did not invalidate itself when the 
image changed. With this fixed there is no need to reframe on a class change 
(none that we know of anyway) so I can fix this without regressing bug 8862. 
Buster showed me the light in the nsBulletFrame code...
Summary: Mousing over certain table cell causes reflow= → Changing element's 'class' causes unnecessary reflow
Fixed. No longer reframing for a class change.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Fixed in the May 30th builds.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: