Closed Bug 364131 Opened 15 years ago Closed 15 years ago

Rename ComputeHorizontalValue to ComputeWidthDependentValue (ditto for height)

Categories

(Core :: Layout, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)

Details

Attachments

(3 files)

Spawned off from bug 363573

Patch coming up...
It seems rather pointless to have the unit as a parameter just to
check it in a debug build in this case...
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsHTMLReflowState.cpp&rev=1.242&root=/cvsroot&mark=188,192#186
Even if the compiler manages to optimize it into a NOP, it adds
unnecessary lines to the code. Most callers use GetUnit() in the
call anyway (or in a few cases, the same stashed in a local var).
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsHTMLReflowState.cpp&rev=1.242&root=/cvsroot&mark=468,469#466
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsHTMLReflowState.cpp&rev=1.242&root=/cvsroot&mark=720,765#707

Should we remove it while we're here...
Attached patch Patch rev. 1Splinter Review
ComputeHorizontalValue => ComputeWidthDependentValue
ComputeVerticalValue => ComputeHeightDependentValue
Remove the unit parameter from the nsCSSOffsetState methods
Attachment #248915 - Flags: superreview?(dbaron)
Attachment #248915 - Flags: review?(dbaron)
Assignee: nobody → mats.palmgren
Comment on attachment 248915 [details] [diff] [review]
Patch rev. 1

r+sr=dbaron

One nit:  In all but one of the changes in nsFrame.cpp (the one for min-width) you redid the indentation pattern.  I slightly preferred it the way it was (so it was clear that the "- boxSizing Adjust" was outside the functyoion call), but whichever way  you go the min-width one should be consistent with the rest.
Attachment #248915 - Flags: superreview?(dbaron)
Attachment #248915 - Flags: superreview+
Attachment #248915 - Flags: review?(dbaron)
Attachment #248915 - Flags: review+
Attached patch nit fixedSplinter Review
Comment on attachment 248994 [details] [diff] [review]
nit fixed

Checked in to trunk at 2006-12-18 05:20 PST

-> FIXED
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.