Closed Bug 762182 Opened 12 years ago Closed 12 years ago

nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-variable]

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: dholbert, Assigned: jhk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

{
../../../../mozilla/content/base/src/nsScriptLoader.cpp: In member function 'bool nsScriptLoader::ProcessScriptElement(nsIScriptElement*)':
../../../../mozilla/content/base/src/nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-variable]
}

Variable was added (& used) here:
https://hg.mozilla.org/mozilla-central/diff/bc10dcdc3b1e/content/base/src/nsScriptLoader.cpp#l1.55

and then its only use was removed in bug 738380, here:
https://hg.mozilla.org/mozilla-central/diff/3eee3ceb400b/content/base/src/nsScriptLoader.cpp#l1.12
leaving us with the build warning.

So it looks like the variable can just be removed.
...though the comment saying that we should rely on |rootElement| is still there.

So bug 738380 left that chunk of code with the comment & code disagreeing. Currently it looks like this:

> 395   // Default script language is whatever the root element specifies
> 396   // (which may come from a header or http-meta tag), or if there
> 397   // is no root element, from the script global object.
> 398   Element* rootElement = mDocument->GetRootElement();
> 399   PRUint32 typeID = nsIProgrammingLanguage::JAVASCRIPT;
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsScriptLoader.cpp#395

Assuming the code is correct & rootElement is safe to remove, then we presumably want to remove that comment, too.
Attached patch PatchSplinter Review
Attachment #630678 - Flags: review?(dholbert)
Comment on attachment 630678 [details] [diff] [review]
Patch

Based on bug 738380 comment 0, it sounds like the comment is obsolete & this patch is fine -- but Ms2ger or bz would know for sure.

Kicking review over to Ms2ger.
Attachment #630678 - Flags: review?(dholbert) → review?(Ms2ger)
Comment on attachment 630678 [details] [diff] [review]
Patch

Yep, this is correct.
Attachment #630678 - Flags: review?(Ms2ger) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/907796393e09
Assignee: nobody → jigneshhk1992
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/907796393e09
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: