Last Comment Bug 783700 - nsColumnSetFrame::BuildDisplayList should use nsFrameList::Enumerator to enumerate children
: nsColumnSetFrame::BuildDisplayList should use nsFrameList::Enumerator to enum...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-17 15:23 PDT by Daniel Holbert [:dholbert]
Modified: 2012-08-20 10:14 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.36 KB, patch)
2012-08-17 15:28 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-08-17 15:23:30 PDT
We should be using the (relatively new) nsFrameList::Enumerator class for frame-list-iteration where possible, instead of directly walking a frame list.

nsColumnSetFrame::BuildDisplayList currently walks directly over its child list. Filing this bug on switching it to using nsFrameList::Enumerator.
Comment 1 Daniel Holbert [:dholbert] 2012-08-17 15:28:56 PDT
Created attachment 652955 [details] [diff] [review]
fix

FWIW, I just cribbed the Enumerator "for" loop line from here:
 http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableRowGroupFrame.cpp#43
Comment 2 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-08-17 16:31:44 PDT
I'm curious whether you guys think the intent was that we use nsFrameList::Enumerator this way.  (I originally suggested it to dholbert, but then he pointed out that code like this wasn't converted when nsFrameList::Enumerator was introduced.)
Comment 3 Mats Palmgren (vacation) 2012-08-17 17:36:52 PDT
Yes, I think the intent was to convert frame list loops in this way.
I certainly do it whenever I get the chance.  Please r- me if you
see I don't :-)
Comment 4 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-08-17 17:40:05 PDT
Comment on attachment 652955 [details] [diff] [review]
fix

r=dbaron
Comment 5 Daniel Holbert [:dholbert] 2012-08-17 18:13:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/53951ff066a6
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-08-18 04:26:26 PDT
https://hg.mozilla.org/mozilla-central/rev/53951ff066a6

Note You need to log in before you can comment on or make changes to this bug.