712299 - don't require manual syncing of font data (kCSSRawFontDescs et al)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Nathan Froyd [:froydnj]]

Comments with nsCSSProps.cpp:kCSSRawFontDescs and elsewhere warn the reader about keeping various bits in sync.  No one list is complete and each list is slightly different.

Make the madness stop!  Patch incoming.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: patch

Simple patch to create nsCSSFontDescList.h and make various places use it.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could you make the parameters to the macro not be uppercase, but instead have a trailing _ character?  That matches the style of other things in nearby code, in particular, nsCSSPropList.h.  Also, since the fields actually match 2 of the fields in the CSS_PROP macro, could you use the same names, so that you have:

#define CSS_FONT_DESC(name_, method_)

instead of:

#define FONT_DESC(CSS_PROP, NAME)

Otherwise, looks great.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 583152 [details] [diff] [review]
patch

In addition to what dbaron said, those trailing commas are likely to be a problem, iirc.

Add nsnull at the end of kCSSRawFontDescs, for sure; I think nsCSSFontFaceStyleDecl::Fields should have one too.

r=me with that.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why does either need a null?  They're only accessed by index, I hope.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Last I checked some compilers warned on the trailing comma.  But maybe not anymore?

Comment 6

[Nathan Froyd [:froydnj]]

We already have trailing commas elsewhere, e.g. nsCSSProps.cpp:kCSSRawProperties.

Comment 7

[Boris Zbarsky [:bzbarsky]]

OK, then don't worry about it.

Comment 8

[Nathan Froyd [:froydnj]]

Attachment: patch v2

Addressed bz and dbaron's comments.  Carrying over r+.

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e4fe4d48518f

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e4fe4d48518f