116674 - Compile warnings for content/html/style/src.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Description

[Peter A Jonsson]

Some compile warnings fixed, patch should be attached.

Comment 1

[Peter A Jonsson]

Attachment: (Av1) Patch for some of the compile warnings.

Comment 2

[Fabian Guisset]

Comment on attachment 62668 [details] [diff] [review]
(Av1) Patch for some of the compile warnings.

great! r=fabian, though I fail to understand what the changes to
nsCSSLoader.cpp do. Oh and if possible, please attach a diff -u next time, it's
much easier to read. Thanks :)

Comment 3

[Johnny Stenback (:jst)]

Reassigning to the style system. In the future (and if you still have the
changes, please attach a new version of the patch), please attach cvs diff -u
format patches as they are much more readable.

Comment 4

[Peter A Jonsson]

Attachment: (Av1b) (diff -u): Patch to fix some of the compile warnings.

Same diff but with -u this time.

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 62669 [details] [diff] [review]
(Av1b) (diff -u): Patch to fix some of the compile warnings.

sr=jst

Comment 6

[Johnny Stenback (:jst)]

Thanks for the new diff.

Comment 7

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

In some of those cases, isn't adding "default: break;" wrong since we really
*should* be handling all the enum values?

Comment 8

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

Also, why bother initializing |units| to zero when we're going to set it anyway?
 Isn't the better fix to change the last |else if| in the chain to an |else|
plus an |NS_ASSERTION|?

Comment 9

[Peter A Jonsson]

Attachment: (Av2) Patch2, second try.

Bad morning, third try:

Two break statements removed, they should not have been there. 

The reordering part should definitely be done, the other questions about the
patch I can not answer.

Comment 10

[Peter A Jonsson]

Closing this bug as WONTFIX, no point in having bugs laying around. 

Comment 11

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

Reopening.

Comment 12

[Peter A Jonsson]

Attachment: (Av2b) Fix some warnings.

This obsoletes the earlier tries. It still leaves a bunch of:

In file included from nsComputedDOMStyle.h:45,
		 from nsComputedDOMStyle.cpp:42:
nsROCSSPrimitiveValue.h: In member function `void 
   nsROCSSPrimitiveValue::SetIdent(const nsACString&)':
nsROCSSPrimitiveValue.h:93: warning: enumeral mismatch in conditional 
   expression: `nsIDOMCSSPrimitiveValue::<anonymous enum>' vs 
   `nsIDOMCSSPrimitiveValue::<anonymous enum>'
nsROCSSPrimitiveValue.h: In member function `void 
   nsROCSSPrimitiveValue::SetIdent(const nsAString&)':

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

I'd forgotten about this bug.  Actually, we don't want to do casts to ints like
that in nsComputedDOMStyle.cpp (nor anywhere in our code really).  I've actually
been working on a patch this morning for that file under the auspices of bug 139343.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

The patch on bug 139343 also takes care of the nsROCSSPrimitiveValue warnings
too, since that is used by computed style.

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 80584 [details] [diff] [review]
(Av2b) Fix some warnings.


There is nothing left to fix :->