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)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
M15
People
(Reporter: mikepinkerton, Assigned: attinasi)
References
()
Details
Attachments
(1 file)
33.07 KB,
text/html
|
Details |
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.
Reporter | ||
Comment 1•25 years ago
|
||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Updated•25 years ago
|
Target Milestone: M13
Comment 2•25 years ago
|
||
It reflows in viewer on WIN32 as well.
Comment 3•25 years ago
|
||
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');"> <A class="white" href="/news.htm"><FONT face=Arial, Helvetica color=#f0f0df size=+1>News & 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.
Comment 4•25 years ago
|
||
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>
Updated•25 years ago
|
Assignee: kmcclusk → troy
Status: ASSIGNED → NEW
Comment 5•25 years ago
|
||
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
Comment 7•24 years ago
|
||
Block-moved to attinasi the bugs related to style inside tables
Assignee: pierre → attinasi
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•24 years ago
|
||
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...
Comment 9•24 years ago
|
||
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
Assignee | ||
Comment 10•24 years ago
|
||
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.
Assignee | ||
Comment 11•24 years ago
|
||
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
Assignee | ||
Comment 12•24 years ago
|
||
Fixed. No longer reframing for a class change.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•