Closed Bug 1280647 Opened 8 years ago Closed 8 years ago

Rename nsStyleDisplay member "mFloats" to "mFloat"

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: dholbert, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The computed value of the "float" property is currently stored in a variable named "mFloats" (with an "s").

This is confusing. Absent a good reason, it should simply be named "mFloat" (with no "s").

HISTORICAL NOTES:
=================
As far as I can tell, the nsStyleDisplay "mFloats" member dates back to this change from kipp:
http://searchfox.org/mozilla-central/diff/f94192041a402c826fb74263af6c111a05af2896/layout/style/nsStyleContext.cpp#315
...with no bug number, and with commit message:
> Implement nsStyleText, nsStyleDisplay; added cursors; removed hack code

That changeset removes lines of code like this:
> -           mMolecule.floats = NS_STYLE_FLOAT_LEFT;
...and replaces it with code like:
> +     mFloats = NS_STYLE_FLOAT_NONE;

Importantly, note that the removed code there didn't have the "m" prefix -- it just called it "floats".  *THAT* variable had a good reason for the "s" suffix - "float" would be an invalid variable name in C++, since it's the type name for a floating-point number. So presumably that variable was called "floats" as a hackaround, and "mFloats" kept the "s" as a vestigial artifact.
Summary: Rename nsStyleDisplay member mFloats" to "mFloat" → Rename nsStyleDisplay member "mFloats" to "mFloat"
Sounds good to me.  This has bothered me in the past.
(tlin / jeremychen, feel free to assign yourself, if you'd like to take this! If it's unclaimed in a week, I may have an intern fix it.)
The mFloats variable in question lives around here:

https://dxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.h#2516
Assignee: nobody → tlin
Also rename mOriginalFloats to mOriginalFloat.

Review commit: https://reviewboard.mozilla.org/r/59566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59566/
Attachment #8763345 - Flags: review?(dholbert)
Comment on attachment 8763345 [details]
Bug 1280647 - Rename nsStyleDisplay member mFloats to mFloat.

https://reviewboard.mozilla.org/r/59566/#review56672

Thanks for noticing and fixing "mOriginalFloats" as well!  It would have been awkward to have the vestigial "s" still sticking around there.

r=me
Attachment #8763345 - Flags: review?(dholbert) → review+
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47e5770b6911
Rename nsStyleDisplay member mFloats to mFloat. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/47e5770b6911
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: