Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in mozilla16

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: jhk)

Tracking

(Blocks: 1 bug)

Trunk
mozilla16
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
{
../../../../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.
(Reporter)

Comment 1

5 years ago
...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.
(Assignee)

Comment 2

5 years ago
Created attachment 630678 [details] [diff] [review]
Patch
Attachment #630678 - Flags: review?(dholbert)
(Reporter)

Comment 3

5 years ago
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+
(Assignee)

Updated

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.