Remove never-actually-set NS_STYLE_CLEAR_* defines from nsStyleConsts.h

RESOLVED FIXED in mozilla23

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We have a list of various #defined internal numeric values for the "clear" property here:

309 // See nsStyleDisplay
310 #define NS_STYLE_CLEAR_NONE                     0
311 #define NS_STYLE_CLEAR_LEFT                     1
312 #define NS_STYLE_CLEAR_RIGHT                    2
313 #define NS_STYLE_CLEAR_LEFT_AND_RIGHT           3
314 #define NS_STYLE_CLEAR_LINE                     4
315 #define NS_STYLE_CLEAR_BLOCK                    5
316 #define NS_STYLE_CLEAR_COLUMN                   6
317 #define NS_STYLE_CLEAR_PAGE                     7
318 #define NS_STYLE_CLEAR_LAST_VALUE NS_STYLE_CLEAR_PAGE

https://mxr.mozilla.org/mozilla-central/source/layout/base/nsStyleConsts.h?rev=13654177590a#309

It looks like NS_STYLE_CLEAR_BLOCK and NS_STYLE_CLEAR_COLUMN are definitely unused (aside from in a debug function that returns a human-readable string label for them):
https://mxr.mozilla.org/mozilla-central/ident?i=NS_STYLE_CLEAR_BLOCK
https://mxr.mozilla.org/mozilla-central/ident?i=NS_STYLE_CLEAR_COLUMN

...and PAGE is also unused, aside from that same debugging function and one assertion that makes sure we don't have PAGE:
https://mxr.mozilla.org/mozilla-central/ident?i=NS_STYLE_CLEAR_PAGE

These #defines date back to 1998:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsStyleConsts.h&mark=271-280#271
so I suspect we may have thought they'd be useful or part of a spec back then, but they've since proved not to be useful.

So: I think we probably want to remove BLOCK, COLUMN, and PAGE.
Posted patch fix v1Splinter Review
Here's the [trivial] patch. Requesting r?mats.

For reference, the spec that defines 'clear' is:
 http://www.w3.org/TR/CSS2/visuren.html#flow-control
(and it has no 'block', 'column', or 'page' values)

Requesting feedback from fantasai as an additional sanity-check, in case she's heard of these keywords being planned for some upcoming CSS spec. (Doubtful, given that these have been in our codebase since 1998 or longer, as noted in comment 0.)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #740931 - Flags: review?(matspal)
Attachment #740931 - Flags: feedback?(fantasai.bugs)
I actually did this as part of the patch for bug 775624, which I never finished off. :)

Where you remove
  NS_ASSERTION(NS_STYLE_CLEAR_PAGE != breakType, "no page breaks yet");
add
  NS_ASSERTION(NS_STYLE_CLEAR_LAST_VALUE >= breakType, "invalid break type");

r=fantasai with that change.
Attachment #740931 - Flags: review?(matspal) → review+
Addressed comment 2 and landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/14929e61959d
OS: Linux → All
Hardware: x86_64 → All
Attachment #740931 - Flags: feedback?(fantasai.bugs) → feedback+
https://hg.mozilla.org/mozilla-central/rev/14929e61959d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.