Closed
Bug 1280647
Opened 8 years ago
Closed 8 years ago
Rename nsStyleDisplay member "mFloats" to "mFloat"
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
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.
Reporter | ||
Updated•8 years ago
|
Summary: Rename nsStyleDisplay member mFloats" to "mFloat" → Rename nsStyleDisplay member "mFloats" to "mFloat"
Sounds good to me. This has bothered me in the past.
status-firefox50:
affected → ---
Reporter | ||
Comment 2•8 years ago
|
||
(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.)
Reporter | ||
Comment 3•8 years ago
|
||
The mFloats variable in question lives around here: https://dxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.h#2516
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → tlin
Assignee | ||
Comment 4•8 years ago
|
||
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)
Reporter | ||
Comment 5•8 years ago
|
||
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
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/47e5770b6911
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•