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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

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.)

Comment 2

[fantasai]

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.

Comment 3

[Daniel Holbert [:dholbert]]

Addressed comment 2 and landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/14929e61959d

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/14929e61959d