Closed Bug 544472 Opened 10 years ago Closed 10 years ago

Improve calls to nsBox::AddCSS***Size

Categories

(Core :: XUL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: enndeakin, Assigned: enndeakin)

References

Details

Attachments

(1 file)

Attached patch changesSplinter Review
These methods are called twice in some cases. This patch removes this and removes some additional unneeded indirection of the other AddCSS*** calls.
Attachment #425455 - Flags: review?(dbaron)
Comment on attachment 425455 [details] [diff] [review]
changes

Given that you removed GetDefaultFlex, I think you should at least make
nsIBox::AddCSSFlex have the part that gets the flex from
aBox->GetStyleXUL() be outside the if(content).  (It's easy to do if you
also put the IsEmpty check outside that if.)  And I tend to think you
should probably do the same for nsIFrame::GetOrdinal.  Though that could
be a separate patch to make regression-hunting easier if you want, on
the off chance it causes problems.  (Feel free to count the r=dbaron on
that too.)

You should also remove nsGroupBoxFrame::GetDefaultFlex (you're removing
the only caller) and nsSprocketLayout::GetDefaultFlex (unused already).



Another potential followup is changing the nsBoxLayoutState& param to
AddCSSMinSize to a nsIRenderingContext*.  (We might at some point need
that on AddCSSPrefSize too if we fix the native theme API to distinguish
min and pref better, which I believe at least Windows can do.)


r=dbaron, and again sorry for taking so long to get to this
Attachment #425455 - Flags: review?(dbaron) → review+
http://hg.mozilla.org/mozilla-central/rev/a35c6813e87d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Depends on: 553814
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.