Last Comment Bug 762182 - nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-variable]
: nsScriptLoader.cpp:398:12: warning: unused variable 'rootElement' [-Wunused-v...
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla16
Assigned To: Jignesh Kakadiya [:jhk]
: Andrew Overholt [:overholt]
Depends on:
Blocks: buildwarning 738380
  Show dependency treegraph
Reported: 2012-06-06 12:11 PDT by Daniel Holbert [:dholbert] (vacation, returning 2/27)
Modified: 2012-06-09 19:44 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (1.20 KB, patch)
2012-06-06 12:57 PDT, Jignesh Kakadiya [:jhk]
Ms2ger: review+
Details | Diff | Splinter Review

Description User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2012-06-06 12:11:45 PDT
../../../../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:

and then its only use was removed in bug 738380, here:
leaving us with the build warning.

So it looks like the variable can just be removed.
Comment 1 User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2012-06-06 12:24:57 PDT
...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;

Assuming the code is correct & rootElement is safe to remove, then we presumably want to remove that comment, too.
Comment 2 User image Jignesh Kakadiya [:jhk] 2012-06-06 12:57:08 PDT
Created attachment 630678 [details] [diff] [review]
Comment 3 User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2012-06-06 13:00:41 PDT
Comment on attachment 630678 [details] [diff] [review]

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.
Comment 4 User image :Ms2ger (⌚ UTC+1/+2) 2012-06-06 13:01:41 PDT
Comment on attachment 630678 [details] [diff] [review]

Yep, this is correct.
Comment 5 User image Ryan VanderMeulen [:RyanVM] 2012-06-09 11:27:02 PDT
Comment 6 User image Ryan VanderMeulen [:RyanVM] 2012-06-09 19:44:10 PDT

Note You need to log in before you can comment on or make changes to this bug.