Closed Bug 116674 Opened 18 years ago Closed 16 years ago

Compile warnings for content/html/style/src.

Categories

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

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.2alpha

People

(Reporter: pj, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

Attachments

(4 obsolete files)

Some compile warnings fixed, patch should be attached.
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 :)
Attachment #62668 - Flags: review+
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.
Assignee: jst → dbaron
Component: DOM Content Models → Style System
QA Contact: stummala → ian
Same diff but with -u this time.
Comment on attachment 62669 [details] [diff] [review]
(Av1b) (diff -u): Patch to fix some of the compile warnings.

sr=jst
Attachment #62669 - Flags: superreview+
Attachment #62669 - Flags: review+
Thanks for the new diff.
Status: UNCONFIRMED → NEW
Ever confirmed: true
In some of those cases, isn't adding "default: break;" wrong since we really
*should* be handling all the enum values?
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|?
Attached patch (Av2) Patch2, second try. (obsolete) — Splinter Review
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.
Closing this bug as WONTFIX, no point in having bugs laying around. 
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WONTFIX
Reopening.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Status: REOPENED → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.1alpha
Attached patch (Av2b) Fix some warnings. (obsolete) — Splinter Review
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&)':
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.
Depends on: 139343
The patch on bug 139343 also takes care of the nsROCSSPrimitiveValue warnings
too, since that is used by computed style.
Target Milestone: mozilla1.1alpha → mozilla1.1beta
Target Milestone: mozilla1.1beta → mozilla1.2alpha
Blocks: buildwarning
Attachment #62668 - Attachment is obsolete: true
Attachment #62669 - Attachment is obsolete: true
Attachment #62677 - Attachment is obsolete: true
Attachment #62668 - Attachment description: Patch for some of the compile warnings. → (Av1) Patch for some of the compile warnings.
Attachment #62669 - Attachment description: (diff -u): Patch to fix some of the compile warnings. → (Av1b) (diff -u): Patch to fix some of the compile warnings.
Attachment #62677 - Attachment description: Patch2, second try. → (Av2) Patch2, second try.
Attachment #80584 - Attachment description: Fix some warnings. → (Av2b) Fix some warnings.
Comment on attachment 80584 [details] [diff] [review]
(Av2b) Fix some warnings.


There is nothing left to fix :->
Attachment #80584 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 18 years ago16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.