Closed Bug 829288 Opened 7 years ago Closed 7 years ago

fix a bunch of mingw warnings

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

No description provided.
Blocks: buildwarning
Attachment #700675 - Flags: review?(dholbert)
Attachment #700677 - Flags: review?(ehsan)
Attachment #700679 - Flags: review?(ehsan)
Attachment #700680 - Flags: review?(gavin.sharp)
Comment on attachment 700677 [details] [diff] [review]
remove extra ';'s in content/

Review of attachment 700677 [details] [diff] [review]:
-----------------------------------------------------------------

FWIW you don't really need to ask review for this kind of stuff!
Attachment #700677 - Flags: review?(ehsan) → review+
Attachment #700678 - Flags: review?(ehsan) → review+
Attachment #700679 - Flags: review?(ehsan) → review+
Attachment #700680 - Flags: review?(gavin.sharp) → review+
Comment on attachment 700675 [details] [diff] [review]
fix layout/ warnings on mingw

># HG changeset patch
># User Trevor Saunders <trev.saunders@gmail.com>
>
>diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp
>index 10350a5..ea3853c 100644
>--- a/layout/base/nsPresContext.cpp
>+++ b/layout/base/nsPresContext.cpp
>@@ -2333,19 +2333,16 @@ enum InterruptMode {
>   ModeRandom,
>   ModeCounter,
>   ModeEvent
> };
> // Controlled by the GECKO_REFLOW_INTERRUPT_MODE env var; allowed values are
> // "random" (except on Windows) or "counter".  If neither is used, the mode is
> // ModeEvent.
> static InterruptMode sInterruptMode = ModeEvent;
>-// Used for the "random" mode.  Controlled by the GECKO_REFLOW_INTERRUPT_SEED
>-// env var.
>-static uint32_t sInterruptSeed = 1;

Looks like you're trying to remove an unused variable there -- but the variable is actually used.  Its only use is in an #ifndef XP_WIN block, though
 https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2364
so you can just add "#ifndef XP_WIN" around the variable instead of dropping it.


>+++ b/layout/style/nsCSSRuleProcessor.cpp
>@@ -1051,18 +1051,18 @@ RuleCascadeData::AttributeListFor(nsIAtom* aAttribute)
> //
> 
> nsCSSRuleProcessor::nsCSSRuleProcessor(const sheet_array_type& aSheets,
>                                        uint8_t aSheetType,
>                                        Element* aScopeElement)
>   : mSheets(aSheets)
>   , mRuleCascades(nullptr)
>   , mLastPresContext(nullptr)
>-  , mSheetType(aSheetType)
>   , mScopeElement(aScopeElement)
>+  , mSheetType(aSheetType)
> {

I actually fixed this one on inbound this morning, in bug 828838. :) So this chunk isn't needed after all. (but thanks for catching it!)

>+++ b/layout/style/nsRuleNode.cpp
>@@ -3053,33 +3053,31 @@ nsRuleNode::SetFont(nsPresContext* aPresContext, nsStyleContext* aContext,
>       // XXXldb This platform-specific stuff should be in the
>       // LookAndFeel implementation, not here.
>       // XXXzw Should we even still *have* this code?  It looks to be making
>       // old, probably obsolete assumptions.
> 
>       // As far as I can tell the system default fonts and sizes
>       // on MS-Windows for Buttons, Listboxes/Comboxes and Text Fields are
>       // all pre-determined and cannot be changed by either the control panel
>-      // or programmtically.
>-      switch (fontID) {
>+      // or programatically.
>         // Fields (text fields)

nit: According to a google search, "programatically" wants two m's -- http://www.thefreedictionary.com/programmatically

Also: I think the the patch leaves this whole chunk indented 2 spaces too much -- could you fix that?

It might also make sense to place the "if" check _before_ the comment that starts with "// Fields (text fields)" -- so then that comment-block wouldn't have to change its indentation -- but it doesn't really matter, I'm happy either way.

>+++ b/layout/style/nsStyleSet.cpp
>@@ -290,17 +290,17 @@ SortStyleSheetsByScope(nsTArray<nsCSSStyleSheet*>& aSheets)
>   }
> }
> 
> nsresult
> nsStyleSet::GatherRuleProcessors(sheetType aType)
> {
>   mRuleProcessors[aType] = nullptr;
>   if (aType == eScopedDocSheet) {
>-    for (int i = 0; i < mScopedDocSheetRuleProcessors.Length(); i++) {
>+    for (uint32_t i = 0; i < mScopedDocSheetRuleProcessors.Length(); i++) {
[etc]
I actually fixed all these nsStyleSet warnings just this morning, too -- bug 829112. :)  So this chunk isn't needed.  Sorry for duplicated-effort there!

r=me with the above addressed.
Attachment #700675 - Flags: review?(dholbert) → review+
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.