Closed Bug 241568 Opened 16 years ago Closed 8 months ago

<nsFrame.cpp> cleanup

Categories

(Core :: Layout, task, trivial)

task
Not set
trivial

Tracking

()

RESOLVED WONTFIX

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

Attachments

(1 file, 1 obsolete file)

Fix for
{{ <http://tinderbox.mozilla.org/SeaMonkey/warn1082809200.7591.html>
anthonyd (2 warnings)
1.	layout/html/base/src/nsFrame.cpp:3879 (See build log excerpt)
	`nsIFrame*thisBlock' might be used uninitialized in this function
2.	layout/html/base/src/nsFrame.cpp:3908 (See build log excerpt)
	`nsIFrame*thisBlock' might be used uninitialized in this function
}} (NB: lines should be lines - 2, due to checkin "in progress")
will be included.
Attached patch (Av1) <nsFrame.cpp> (obsolete) — Splinter Review
Also removes:
{{ <http://lxr.mozilla.org/mozilla/search?string=RefreshContentFrames>
RefreshContentFrames
/layout/html/base/src/nsFrame.cpp, line 121 -- static void
RefreshContentFrames(nsIPresContext* aPresContext, nsIContent * aStartContent,
nsIContent * aEndContent);
}}

(There could be more cleanup to do: but enough for now.)
Assignee: nobody → gautheri
Status: NEW → ASSIGNED
Comment on attachment 146923 [details] [diff] [review]
(Av1) <nsFrame.cpp>


I have no compiler: Could you compile/test/review this patch ? Thanks.
Attachment #146923 - Flags: review?(rbs)
Note that nsFrame.cpp has nothing to do with HTML Frames, and I'm a little wary
of anyone who doesn't know that touching this file....
I already reviewed a patch (by the same author) for the warnings in comment 0 in
another bug.

Code cleanup and warning fixes are really only beneficial to the maintainers of
the code in question.  So is cvs blame.  Things like this mess up cvs blame, and
I'm not sure the benefits are worth it.
Component: Layout: HTML Frames → Layout: Misc Code
(In reply to comment #4)
> I already reviewed a patch (by the same author) for the warnings in comment 0 in
> another bug.

Are you sure ?
May be that was an other warning from bug 217089:
<http://bugzilla.mozilla.org/attachment.cgi?id=146892&action=view> !?

> Code cleanup and warning fixes are really only beneficial to the maintainers of
> the code in question.  So is cvs blame.  Things like this mess up cvs blame, and
> I'm not sure the benefits are worth it.

Well, there are some improvements too: let's wait for rbs's review...
Attachment #146923 - Attachment is obsolete: true
Attachment #146923 - Flags: review?(rbs)
Av1, "improved".
Attachment #147072 - Flags: review?(rbs)
Comment on attachment 147072 [details] [diff] [review]
(Av1b) <nsFrame.cpp>

I am not really keen on this one, as it came from the coding style of the
initial coder. It is only worth the time/effort if you are taking ownership of
the file and asserting you style. [Note that the compiler will optimize most
away -- except, e.g., that |if view| with multiple calls -- which isn't
significant here]).
Attachment #147072 - Flags: review?(rbs)
QA Contact: layout.html-frames → layout.misc-code
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core

The code probably changed a lot in 10 years, closing

Status: ASSIGNED → RESOLVED
Type: defect → task
Closed: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.