Fix compiler warnings: "unused variable" and "defined but not used"

RESOLVED FIXED

Status

()

Core
General
--
trivial
RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: Matthias Bockelkamp, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning] bugday0420)

Attachments

(51 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030820
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030820

I'll attach a few patches for "warning: unused variable" and "... defined but
not used" in different files using the MinGW compiler.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Comment 1

14 years ago
Created attachment 130273 [details] [diff] [review]
Patch for CNavDTD.cpp
(Reporter)

Updated

14 years ago
Attachment #130273 - Flags: superreview?(jst)
Attachment #130273 - Flags: review?(jst)
(Reporter)

Comment 2

14 years ago
Created attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]
(Reporter)

Updated

14 years ago
Attachment #130274 - Flags: superreview?(beard)
Attachment #130274 - Flags: review?(beard)
(Reporter)

Comment 3

14 years ago
Created attachment 130275 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v1
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

14 years ago
Attachment #130275 - Flags: superreview?(scc)
Attachment #130275 - Flags: review?(scc)
(Reporter)

Comment 4

14 years ago
Created attachment 130276 [details] [diff] [review]
Patch for lcglue.cpp
(Reporter)

Updated

14 years ago
Attachment #130276 - Flags: superreview?(scc)
Attachment #130276 - Flags: review?(scc)
(Reporter)

Comment 5

14 years ago
Created attachment 130277 [details] [diff] [review]
Patch for ns4xPlugin.cpp
(Reporter)

Updated

14 years ago
Attachment #130277 - Flags: superreview?(beard)
Attachment #130277 - Flags: review?(beard)
(Reporter)

Comment 6

14 years ago
Created attachment 130278 [details] [diff] [review]
Patch for nsBoxObject.cpp
(Reporter)

Updated

14 years ago
Attachment #130278 - Flags: superreview?(bryner)
Attachment #130278 - Flags: review?(bryner)
(Reporter)

Comment 7

14 years ago
Created attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]
(Reporter)

Updated

14 years ago
Attachment #130280 - Flags: superreview?(blizzard)
Attachment #130280 - Flags: review?(blizzard)
(Reporter)

Comment 8

14 years ago
Created attachment 130281 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1
(Reporter)

Updated

14 years ago
Attachment #130281 - Flags: superreview?(blizzard)
Attachment #130281 - Flags: review?(blizzard)
(Reporter)

Comment 9

14 years ago
Created attachment 130283 [details] [diff] [review]
Patch for nsDocumentViewer.cpp
(Reporter)

Updated

14 years ago
Attachment #130283 - Flags: superreview?(bz-vacation)
Attachment #130283 - Flags: review?(bz-vacation)
(Reporter)

Comment 10

14 years ago
Created attachment 130284 [details] [diff] [review]
<nsFileSystemDataSource.cpp>
[Checked in: Comment 77]
(Reporter)

Updated

14 years ago
Attachment #130284 - Flags: superreview?(scc)
Attachment #130284 - Flags: review?(scc)
(Reporter)

Comment 11

14 years ago
Created attachment 130286 [details] [diff] [review]
Patch for nsFontMetricsWin.cpp
(Reporter)

Updated

14 years ago
Attachment #130286 - Flags: superreview?(blizzard)
Attachment #130286 - Flags: review?(blizzard)
(Reporter)

Comment 12

14 years ago
Created attachment 130287 [details] [diff] [review]
</gfx/src/windows/nsGfxFactoryWin.cpp>
(Reporter)

Updated

14 years ago
Attachment #130287 - Flags: superreview?(blizzard)
Attachment #130287 - Flags: review?(blizzard)
(Reporter)

Comment 13

14 years ago
Created attachment 130288 [details] [diff] [review]
Patch for nsHTMLContentSink.cpp
(Reporter)

Updated

14 years ago
Attachment #130288 - Flags: superreview?(bz-vacation)
Attachment #130288 - Flags: review?(bz-vacation)
(Reporter)

Comment 14

14 years ago
Created attachment 130289 [details] [diff] [review]
Patch for nsImageWin.cpp
(Reporter)

Updated

14 years ago
Attachment #130289 - Flags: superreview?(blizzard)
Attachment #130289 - Flags: review?(blizzard)
(Reporter)

Comment 15

14 years ago
Created attachment 130290 [details] [diff] [review]
Patch for nsJVMManager.cpp
(Reporter)

Updated

14 years ago
Attachment #130290 - Flags: superreview?(scc)
Attachment #130290 - Flags: review?(scc)
(Reporter)

Comment 16

14 years ago
Created attachment 130291 [details] [diff] [review]
</widget/src/windows/nsNativeDragTarget.cpp> [Checked in: Comment 173]
(Reporter)

Updated

14 years ago
Attachment #130291 - Flags: superreview?(blizzard)
Attachment #130291 - Flags: review?(blizzard)
(Reporter)

Comment 17

14 years ago
Created attachment 130292 [details] [diff] [review]
</gfx/src/windows/nsNativeThemeWin.cpp>
[Checked in: Comment 135]
(Reporter)

Updated

14 years ago
Attachment #130292 - Flags: superreview?(blizzard)
Attachment #130292 - Flags: review?(blizzard)
(Reporter)

Comment 18

14 years ago
Created attachment 130294 [details] [diff] [review]
Patch for nsOSHelperAppService.cpp
(Reporter)

Updated

14 years ago
Attachment #130294 - Flags: superreview?(scott)
Attachment #130294 - Flags: review?(scott)
(Reporter)

Comment 19

14 years ago
Created attachment 130295 [details] [diff] [review]
</modules/plugin/base/src/nsPluginsDirWin.cpp>
[Checked in: Comment 155]
(Reporter)

Updated

14 years ago
Attachment #130295 - Flags: superreview?(beard)
Attachment #130295 - Flags: review?(beard)
(Reporter)

Comment 20

14 years ago
Created attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]
(Reporter)

Updated

14 years ago
Attachment #130296 - Flags: superreview?(blizzard)
Attachment #130296 - Flags: review?(blizzard)
(Reporter)

Comment 21

14 years ago
Created attachment 130297 [details] [diff] [review]
Patch for nsRDFXMLDataSource.cpp
[Checked in: Comment 112]
(Reporter)

Updated

14 years ago
Attachment #130297 - Flags: superreview?(scc)
Attachment #130297 - Flags: review?(scc)
(Reporter)

Comment 22

14 years ago
Created attachment 130298 [details] [diff] [review]
Patch for nsScanner.cpp
(Reporter)

Updated

14 years ago
Attachment #130298 - Flags: superreview?(jst)
Attachment #130298 - Flags: review?(jst)
(Reporter)

Comment 23

14 years ago
Created attachment 130299 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp
(Reporter)

Updated

14 years ago
Attachment #130299 - Flags: superreview?(blizzard)
Attachment #130299 - Flags: review?(blizzard)
(Reporter)

Comment 24

14 years ago
Created attachment 130300 [details] [diff] [review]
Patch for nsSelection.cpp
(Reporter)

Updated

14 years ago
Attachment #130300 - Flags: superreview?(bz-vacation)
Attachment #130300 - Flags: review?(bz-vacation)
(Reporter)

Comment 25

14 years ago
Created attachment 130301 [details] [diff] [review]
Patch for nsToolkit.cpp
(Reporter)

Updated

14 years ago
Attachment #130301 - Flags: superreview?(bryner)
Attachment #130301 - Flags: review?(bryner)
(Reporter)

Comment 26

14 years ago
Created attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]
(Reporter)

Updated

14 years ago
Attachment #130302 - Flags: superreview?(blizzard)
Attachment #130302 - Flags: review?(blizzard)
(Reporter)

Comment 27

14 years ago
Created attachment 130303 [details] [diff] [review]
Patch for nsWindow.cpp
(Reporter)

Updated

14 years ago
Attachment #130303 - Flags: superreview?(blizzard)
Attachment #130303 - Flags: review?(blizzard)
(Reporter)

Comment 28

14 years ago
Created attachment 130304 [details] [diff] [review]
Patch for nsXMLContentSink.cpp
(Reporter)

Updated

14 years ago
Attachment #130304 - Flags: superreview?(hyatt)
Attachment #130304 - Flags: review?(hyatt)
(Reporter)

Comment 29

14 years ago
Created attachment 130305 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v1
(Reporter)

Updated

14 years ago
Attachment #130305 - Flags: superreview?(bryner)
Attachment #130305 - Flags: review?(bryner)
(Reporter)

Comment 30

14 years ago
Created attachment 130306 [details] [diff] [review]
</modules/plugin/samples/default/windows/plugin.cpp> v1
(Reporter)

Updated

14 years ago
Attachment #130306 - Flags: superreview?(beard)
Attachment #130306 - Flags: review?(beard)
Comment on attachment 130305 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v1

There's a call to this function in nsXULTooltipListener::LaunchTooltip, #ifdef
DEBUG_crap.  I'm not quite sure what the status is of that #ifdef... jag, you
tweaked it a bit, are you using it for something?
Attachment #130305 - Flags: superreview?(bryner) → superreview?(jag)
Attachment #130301 - Flags: superreview?(bryner)
Attachment #130301 - Flags: superreview+
Attachment #130301 - Flags: review?(bryner)
Attachment #130301 - Flags: review+
Comment on attachment 130278 [details] [diff] [review]
Patch for nsBoxObject.cpp

Uh, no.  You can't just remove the call to GetPrimaryFrameFor, as it fills in
|frame|.  Just remove the "nsresult rv = " instead.
Attachment #130278 - Flags: superreview?(bryner)
Attachment #130278 - Flags: superreview-
Attachment #130278 - Flags: review?(bryner)
Attachment #130278 - Flags: review-
(Reporter)

Comment 33

14 years ago
Created attachment 130314 [details] [diff] [review]
Patch for nsBoxObject.cpp, v2
Attachment #130278 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130314 - Flags: superreview?(bryner)
Attachment #130314 - Flags: review?(bryner)
-> mbockelkamp@web.de 
Assignee: general → mbockelkamp

Comment 35

14 years ago
please stop for a while.

Comment 36

14 years ago
Comment on attachment 130275 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v1

Functions have side effects, in this case GetRunningJVM() is *responsible* for
starting the jvm. so you're making the startupjvm function never start a jvm.
Attachment #130275 - Flags: superreview?(scc)
Attachment #130275 - Flags: review?(scc)
Attachment #130275 - Flags: review-

Comment 37

14 years ago
Comment on attachment 130277 [details] [diff] [review]
Patch for ns4xPlugin.cpp

this sure looks like an exported function. don't go killing things that could
be used externally.
Attachment #130277 - Flags: superreview?(beard)
Attachment #130277 - Flags: review?(beard)
Attachment #130277 - Flags: review-

Comment 38

14 years ago
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]

fwiw pinkerton removed the consumer in rev 1.53
Attachment #130280 - Flags: superreview?(sfraser)
Attachment #130280 - Flags: superreview?(blizzard)
Attachment #130280 - Flags: review?(pinkerton)
Attachment #130280 - Flags: review?(blizzard)

Comment 39

14 years ago
Comment on attachment 130304 [details] [diff] [review]
Patch for nsXMLContentSink.cpp

look at the patch, see how ref is used two statements later?
Attachment #130304 - Flags: superreview?(hyatt)
Attachment #130304 - Flags: review?(hyatt)
Attachment #130304 - Flags: review-

Comment 40

14 years ago
Comment on attachment 130306 [details] [diff] [review]
</modules/plugin/samples/default/windows/plugin.cpp> v1

well, i wouldn't mind killing plugin finder, but this isn't the way to do it.
Attachment #130306 - Flags: superreview?(beard)
Attachment #130306 - Flags: review?(beard)
Attachment #130306 - Flags: review-

Comment 41

14 years ago
he future, please use cvs to generate diffs, cvs diff -up5 or something would be
good. Ideally for small functions you should diff enough to show that a local
variable really isn't used.

also if you're going to split patches up, you should pick people who have done
stuff in the areas recently.

however some people think you should put together a single patch and get a big
overall review.

i'm not sure i agree, since i think that only a few people would recognize the
damage your patches would cause. unfortunately they aren't the people you
selected for reviews.

Updated

14 years ago
Attachment #130273 - Flags: superreview?(jst)
Attachment #130273 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130273 - Flags: review?(jst)
Attachment #130273 - Flags: review+
(Reporter)

Comment 42

14 years ago
Created attachment 130327 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v2
[Checked in: Comment 166]
Attachment #130275 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130327 - Flags: superreview?(timeless)
Attachment #130327 - Flags: review?(timeless)
(Reporter)

Comment 43

14 years ago
Created attachment 130328 [details] [diff] [review]
Patch for nsXMLContentSink.cpp, v2
Attachment #130304 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130328 - Flags: superreview?(timeless)
Attachment #130328 - Flags: review?(timeless)
(Reporter)

Comment 44

14 years ago
Created attachment 130329 [details] [diff] [review]
<plugin.cpp>, v2
Attachment #130306 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130329 - Flags: superreview?(timeless)
Attachment #130329 - Flags: review?(timeless)
(Reporter)

Updated

14 years ago
Attachment #130277 - Attachment is obsolete: true

Comment 45

14 years ago
http://www.mozilla.org/hacking/reviewers.html lists people who are srs and the
areas in which they specialize, i'm not on the list, so i can't sr.
(Reporter)

Updated

14 years ago
Attachment #130327 - Flags: superreview?(timeless)
Attachment #130327 - Flags: superreview?(hyatt)
Attachment #130327 - Flags: review?(timeless)
Attachment #130327 - Flags: review?(hyatt)
(Reporter)

Updated

14 years ago
Attachment #130328 - Flags: superreview?(timeless)
Attachment #130328 - Flags: superreview?(hyatt)
Attachment #130328 - Flags: review?(timeless)
Attachment #130328 - Flags: review?(hyatt)
(Reporter)

Updated

14 years ago
Attachment #130329 - Flags: superreview?(timeless)
Attachment #130329 - Flags: superreview?(hyatt)
Attachment #130329 - Flags: review?(timeless)
Attachment #130329 - Flags: review?(hyatt)
(Reporter)

Updated

14 years ago
Attachment #130327 - Flags: superreview?(scc)
Attachment #130327 - Flags: superreview?(hyatt)
Attachment #130327 - Flags: review?(scc)
Attachment #130327 - Flags: review?(hyatt)
(Reporter)

Updated

14 years ago
Attachment #130329 - Flags: superreview?(hyatt)
Attachment #130329 - Flags: superreview?(beard)
Attachment #130329 - Flags: review?(hyatt)
Attachment #130329 - Flags: review?(beard)

Comment 46

14 years ago
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]

this change should be made to both windows and os2 plugins.
Attachment #130274 - Flags: review?(beard) → review+

Updated

14 years ago
Attachment #130276 - Flags: superreview?(scc)
Attachment #130276 - Flags: superreview?(jst)
Attachment #130276 - Flags: review?(scc)
Attachment #130276 - Flags: review?(mkaply)

Updated

14 years ago
Attachment #130283 - Flags: superreview?(dbaron)
Attachment #130283 - Flags: superreview?(bz-vacation)
Attachment #130283 - Flags: review?(bz-vacation)
Attachment #130283 - Flags: review+

Updated

14 years ago
Attachment #130284 - Flags: superreview?(scc)
Attachment #130284 - Flags: superreview?(alecf)
Attachment #130284 - Flags: review?(scc)
Attachment #130284 - Flags: review+

Updated

14 years ago
Attachment #130286 - Flags: superreview?(rbs)
Attachment #130286 - Flags: superreview?(blizzard)
Attachment #130286 - Flags: review?(rbs)
Attachment #130286 - Flags: review?(blizzard)
(Reporter)

Comment 47

14 years ago
Created attachment 130332 [details] [diff] [review]
<os2/dialogs.cpp>
[Checked in: Comment 107]
(Reporter)

Updated

14 years ago
Attachment #130332 - Flags: superreview?(beard)
Attachment #130332 - Flags: review?(timeless)
(Reporter)

Updated

14 years ago
Attachment #130274 - Attachment description: Patch for dialogs.cpp → Patch for dialogs.cpp (win)

Comment 48

14 years ago
Comment on attachment 130287 [details] [diff] [review]
</gfx/src/windows/nsGfxFactoryWin.cpp>

personally i think this is the wrong fix.

hyatt: why do you need NS_NewNativeTheme? why not use the generic factory
constructor.

i'd rather get rid of the extra exported function and use the normal factory
system.
Attachment #130287 - Flags: superreview?(hyatt)
Attachment #130287 - Flags: superreview?(blizzard)
Attachment #130287 - Flags: review?(hyatt)
Attachment #130287 - Flags: review?(blizzard)

Comment 49

14 years ago
Comment on attachment 130288 [details] [diff] [review]
Patch for nsHTMLContentSink.cpp

this is paired with code in 4093-4102

if you're going to mess with the code please locate pairs and deal equally with
them.
Attachment #130288 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130288 - Flags: superreview?(bz-vacation)
Attachment #130288 - Flags: review?(bz-vacation)
Attachment #130288 - Flags: review-

Updated

14 years ago
Attachment #130289 - Flags: superreview?(roc+moz)
Attachment #130289 - Flags: superreview?(blizzard)
Attachment #130289 - Flags: review?(blizzard)
Attachment #130289 - Flags: review+

Comment 50

14 years ago
Comment on attachment 130290 [details] [diff] [review]
Patch for nsJVMManager.cpp

#include <stdio.h>
int a(char c,int d){
printf("hello world [%c:%d]\n",c,d);return 1;
}
int main(void){
for(int i=0;i<5;i++)
{static int y=a('s',i);int z=a('!',i);}
return 0;
}

compile this c++ program. you're changing the code from 's' to '!'.
Attachment #130290 - Flags: superreview?(scc)
Attachment #130290 - Flags: review?(scc)
Attachment #130290 - Flags: review-

Updated

14 years ago
Attachment #130291 - Flags: superreview?(hyatt)
Attachment #130291 - Flags: superreview?(blizzard)
Attachment #130291 - Flags: review?(blizzard)
Attachment #130291 - Flags: review+
(Reporter)

Comment 51

14 years ago
Created attachment 130336 [details] [diff] [review]
Patch for nsHTMLContentSink.cpp, v2
Attachment #130288 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130336 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130336 - Flags: review?(timeless)
(Reporter)

Updated

14 years ago
Attachment #130288 - Flags: superreview?(hjtoi-bugzilla)
(Reporter)

Comment 52

14 years ago
Comment on attachment 130290 [details] [diff] [review]
Patch for nsJVMManager.cpp

> compile this c++ program. you're changing the code from 's' to '!'.

OK. I didn't understand it until I saw its output.
Attachment #130290 - Attachment is obsolete: true

Comment 53

14 years ago
Comment on attachment 130292 [details] [diff] [review]
</gfx/src/windows/nsNativeThemeWin.cpp>
[Checked in: Comment 135]

i can't imagine that we would want to remove the prototypes.

for my reference because you didn't include enough context, isDisabled probably
was meant to store IsDisabled(aFrame). since it is just used inline w/ ifs it
makes sense to remove the PRBools.
Attachment #130292 - Flags: superreview?(hyatt)
Attachment #130292 - Flags: superreview?(blizzard)
Attachment #130292 - Flags: review?(tim)
Attachment #130292 - Flags: review?(blizzard)

Updated

14 years ago
Attachment #130294 - Flags: superreview?(scott)
Attachment #130294 - Flags: superreview?(darin)
Attachment #130294 - Flags: review?(scott)
Attachment #130294 - Flags: review+

Comment 54

14 years ago
Comment on attachment 130295 [details] [diff] [review]
</modules/plugin/base/src/nsPluginsDirWin.cpp>
[Checked in: Comment 155]

note to self: info.fName in the beos version mismatches allocator/deallocators
(for some paths)
Attachment #130295 - Flags: review?(beard) → review+

Comment 55

14 years ago
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]

note to self: that assertion is bogus, it should be replaced with:
if (!printSettings) return NS_ERROR_OUT_OF_MEMORY;
(for all the impls)
Attachment #130296 - Flags: superreview?(kin)
Attachment #130296 - Flags: superreview?(blizzard)
Attachment #130296 - Flags: review?(blizzard)
Attachment #130296 - Flags: review+

Comment 56

14 years ago
Comment on attachment 130297 [details] [diff] [review]
Patch for nsRDFXMLDataSource.cpp
[Checked in: Comment 112]

RDFXMLDataSourceImpl::rdfXMLFlush
Attachment #130297 - Flags: superreview?(scc)
Attachment #130297 - Flags: superreview?(darin)
Attachment #130297 - Flags: review?(scc)
Attachment #130297 - Flags: review+

Comment 57

14 years ago
Comment on attachment 130297 [details] [diff] [review]
Patch for nsRDFXMLDataSource.cpp
[Checked in: Comment 112]

RDFXMLDataSourceImpl::rdfXMLFlush

Comment 58

14 years ago
Comment on attachment 130298 [details] [diff] [review]
Patch for nsScanner.cpp

Peek() changes theChar, you can't not call it.
Attachment #130298 - Flags: superreview?(jst)
Attachment #130298 - Flags: review?(jst)
Attachment #130298 - Flags: review-

Comment 59

14 years ago
Comment on attachment 130299 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp

how about making the code work w/ mingw instead?
Attachment #130299 - Flags: superreview?(blizzard)
Attachment #130299 - Flags: review?(blizzard)
Attachment #130299 - Flags: review-

Updated

14 years ago
Attachment #130300 - Flags: superreview?(roc+moz)
Attachment #130300 - Flags: superreview?(bz-vacation)
Attachment #130300 - Flags: review?(bz-vacation)
Attachment #130300 - Flags: review+

Comment 60

14 years ago
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]

i'm not certain the rv should be ignored...
Attachment #130302 - Flags: review?(blizzard) → review?(smontagu)

Comment 61

14 years ago
Comment on attachment 130303 [details] [diff] [review]
Patch for nsWindow.cpp

are we supposed to do something w/ the HCURSOR/HPALETTE from
SetCursor/SelectPalette?
Attachment #130303 - Flags: superreview?(roc+moz)
Attachment #130303 - Flags: superreview?(blizzard)
Attachment #130303 - Flags: review?(ere)
Attachment #130303 - Flags: review?(blizzard)
Comment on attachment 130276 [details] [diff] [review]
Patch for lcglue.cpp

r+sr=jst
Attachment #130276 - Flags: superreview?(jst)
Attachment #130276 - Flags: superreview+
Attachment #130276 - Flags: review?(mkaply)
Attachment #130276 - Flags: review+

Updated

14 years ago
Attachment #130328 - Flags: superreview?(hyatt)
Attachment #130328 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130328 - Flags: review?(hyatt)
Attachment #130328 - Flags: review?(hjtoi-bugzilla)

Updated

14 years ago
Attachment #130332 - Flags: review?(timeless) → review?(mkaply)

Comment 63

14 years ago
Comment on attachment 130336 [details] [diff] [review]
Patch for nsHTMLContentSink.cpp, v2

i don't know if someone might want this code.
Attachment #130336 - Flags: review?(timeless) → review?(jst)
Attachment #130273 - Flags: superreview?(hjtoi-bugzilla) → superreview+
(Reporter)

Comment 64

14 years ago
Created attachment 130349 [details] [diff] [review]
Patch for nsScanner.cpp, v2
Attachment #130298 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130349 - Flags: superreview?(jst)
Attachment #130349 - Flags: review?(timeless)
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]

This looks OK to me: in all cases where a failure code is returned by
MapToCharset() a sensible default is set.

r=smontagu
Attachment #130302 - Flags: review?(smontagu) → review+
Comment on attachment 130336 [details] [diff] [review]
Patch for nsHTMLContentSink.cpp, v2

r+sr=jst
Attachment #130336 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130336 - Flags: superreview+
Attachment #130336 - Flags: review?(jst)
Attachment #130336 - Flags: review+
Comment on attachment 130349 [details] [diff] [review]
Patch for nsScanner.cpp, v2

r+sr=jst
Attachment #130349 - Flags: superreview?(jst)
Attachment #130349 - Flags: superreview+
Attachment #130349 - Flags: review?(timeless)
Attachment #130349 - Flags: review+
Comment on attachment 130283 [details] [diff] [review]
Patch for nsDocumentViewer.cpp

sr=dbaron if you remove the 

#ifdef DEBUG
#defined EXTENDED_DEBUG_PRINTING
#endif

as well, since that should now be in a header file (although it happens to be
copied into two other cpp files instead) -- but in any case it's not needed
here.
Attachment #130283 - Flags: superreview?(dbaron) → superreview+
(Reporter)

Comment 69

14 years ago
Created attachment 130352 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v2

It compiles fine, but I cannot test it with my single monitor equipment.
Attachment #130299 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #130352 - Flags: superreview?(blizzard)
Attachment #130352 - Flags: review?(timeless)
(Reporter)

Comment 70

14 years ago
Created attachment 130353 [details] [diff] [review]
Patch for nsDocumentViewer.cpp, v2

Changed as dbaron said.
r=timeless, sr=dbaron from previous patch.
Attachment #130283 - Attachment is obsolete: true

Comment 71

14 years ago
Comment on attachment 130286 [details] [diff] [review]
Patch for nsFontMetricsWin.cpp

r+sr=rbs
Attachment #130286 - Flags: superreview?(rbs)
Attachment #130286 - Flags: superreview+
Attachment #130286 - Flags: review?(rbs)
Attachment #130286 - Flags: review+

Comment 72

14 years ago
Comment on attachment 130303 [details] [diff] [review]
Patch for nsWindow.cpp

I think it's ok not to care about the results of SetCursor/SelectPalette, but
nsServiceManager::GetService definitely needs error checking. Although the
return value wasn't checked before either, I don't like it being completely
disregarded.
Attachment #130303 - Flags: review?(ere) → review-
Comment on attachment 130300 [details] [diff] [review]
Patch for nsSelection.cpp

Note that jst has this fixed as part of his patch for bug 215891.
I meant bug 215981.

Comment 75

14 years ago
Comment on attachment 130332 [details] [diff] [review]
<os2/dialogs.cpp>
[Checked in: Comment 107]

r=mkaply
sr=blizzard (platform specific code)
Attachment #130332 - Flags: superreview?(beard)
Attachment #130332 - Flags: superreview+
Attachment #130332 - Flags: review?(mkaply)
Attachment #130332 - Flags: review+
(Reporter)

Updated

14 years ago
Attachment #130303 - Attachment is obsolete: true
Attachment #130303 - Flags: superreview?(roc+moz)
(Reporter)

Updated

14 years ago
Attachment #130300 - Attachment is obsolete: true
Attachment #130300 - Flags: superreview?(roc+moz)
Attachment #130289 - Flags: superreview?(roc+moz) → superreview+

Comment 76

14 years ago
Comment on attachment 130305 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v1

That's the code we'll need for making tooltips for truncated text in trees
become titletips. *shrug*, I guess it can be removed, bonsai will remember it
for us if/when we fix that.
Attachment #130305 - Flags: superreview?(jag) → superreview+

Updated

14 years ago
Attachment #130280 - Flags: superreview?(sfraser) → superreview+

Updated

14 years ago
Attachment #130294 - Flags: superreview?(darin) → superreview+

Updated

14 years ago
Attachment #130297 - Flags: superreview?(darin) → superreview+

Comment 77

14 years ago
Comment on attachment 130284 [details] [diff] [review]
<nsFileSystemDataSource.cpp>
[Checked in: Comment 77]

sr=alecf
Attachment #130284 - Flags: superreview?(alecf) → superreview+
Why do you want to remove some of the old MSVC support?  Have we dropped support
for those older compilers?
(Reporter)

Comment 79

14 years ago
Created attachment 131036 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v3
Attachment #130352 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #131036 - Flags: superreview?(blizzard)
Attachment #131036 - Flags: review?(timeless)
(Reporter)

Updated

14 years ago
Attachment #130352 - Flags: superreview?(blizzard)
Attachment #130352 - Flags: review?(timeless)

Comment 80

14 years ago
Comment on attachment 131036 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v3

>-#if _MSC_VER >= 1200
>+#if defined(__MINGW32__) || _MSC_VER >= 1200
> typedef HMONITOR (WINAPI *MonitorFromRectProc)(LPCRECT inRect, DWORD inFlag); 
> typedef BOOL (WINAPI *EnumDisplayMonitorsProc)(HDC, LPCRECT, MONITORENUMPROC, LPARAM);
> 
> BOOL CALLBACK CountMonitors ( HMONITOR, HDC, LPRECT, LPARAM ioCount ) ;
this change isn't a correct transformation:
>-#elif !defined(__MINGW32__)
>-typedef void* HMONITOR;
> #endif

you want
#else
Attachment #131036 - Flags: superreview?(blizzard)
Attachment #131036 - Flags: review?(timeless)
Attachment #131036 - Flags: review-

Updated

14 years ago
Attachment #130273 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130276 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130284 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130286 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130289 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130297 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130301 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130332 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130336 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130349 - Attachment is obsolete: true

Updated

14 years ago
Attachment #130353 - Flags: superreview?(jst)
Attachment #130353 - Flags: review+
(Reporter)

Comment 81

14 years ago
Created attachment 131230 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v4
Attachment #131036 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #131230 - Flags: superreview?(blizzard)
Attachment #131230 - Flags: review?(timeless)

Comment 82

14 years ago
Comment on attachment 131230 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v4

>-#elif !defined(__MINGW32__)
>+#elif
#elif expects a condition, this should just be "#else"
Attachment #131230 - Flags: superreview?(blizzard)
Attachment #131230 - Flags: review?(timeless)
Attachment #131230 - Flags: review-
(Reporter)

Comment 83

14 years ago
Created attachment 131244 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v5
(Reporter)

Updated

14 years ago
Attachment #131230 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #131244 - Flags: superreview?(blizzard)
Attachment #131244 - Flags: review?(timeless)

Updated

14 years ago
Attachment #130353 - Flags: superreview?(jst) → superreview+
Attachment #130305 - Flags: review?(bryner) → review+
Attachment #130314 - Flags: superreview?(bryner)
Attachment #130314 - Flags: superreview+
Attachment #130314 - Flags: review?(bryner)
Attachment #130314 - Flags: review+
Comment on attachment 130294 [details] [diff] [review]
Patch for nsOSHelperAppService.cpp

this patch is checked in:
09/10/2003 20:18	timeless%mozdev.org	mozilla/ uriloader/ exthandler/
win/ nsOSHelperAppService.cpp	1.42	0/2	Bug 217089 Compiler warnings
patch by mbockelkamp@web.de r=timeless sr=darin
Attachment #130294 - Attachment is obsolete: true

Comment 85

14 years ago
Comment on attachment 131244 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v5

>@@ -225,15 +225,15 @@
> nsScreenManagerWin :: GetNumberOfScreens(PRUint32 *aNumberOfScreens)
(EnumDisplayMonitorsProc)mEnumDisplayMonitorsProc;
>-      BOOL result = (*proc)(nsnull, nsnull, (MONITORENUMPROC)CountMonitors, (LPARAM)&count);
>+      (*proc)(nsnull, nsnull, (MONITORENUMPROC)CountMonitors, (LPARAM)&count);
>       *aNumberOfScreens = mNumberOfScreens = count;      

I'm very uncomfortable using a value in a failure case. the winapi doesn't
suggest how it might fail, but does of course allow it to fail. mozilla xpcom
is the same in that respect and for xpcom failures you aren't supposed to use 
values if the function failed...
Attachment #131244 - Flags: review?(timeless) → review?(ere)

Comment 86

14 years ago
Comment on attachment 131244 [details] [diff] [review]
Patch for nsScreenManagerWin.cpp, v5

Timeless is right. The function could fail and the correct fix is to check the
return value and act accordingly.
Attachment #131244 - Flags: superreview?(blizzard)
Attachment #131244 - Flags: review?(ere)
Attachment #131244 - Flags: review-
Attachment #130328 - Flags: superreview?(hjtoi-bugzilla)
Attachment #130328 - Flags: superreview+
Attachment #130328 - Flags: review?(hjtoi-bugzilla)
Attachment #130328 - Flags: review+
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]

if you say this isn't used, i guess i believe you

r=pink
Attachment #130280 - Flags: review?(pinkerton) → review+

Comment 88

14 years ago
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]

sr=kinmoz@netscape.net
Attachment #130296 - Flags: superreview?(kinmoz) → superreview+
Attachment #130284 - Attachment description: Patch for nsFileSystemDataSource.cpp → <nsFileSystemDataSource.cpp> [Checked in: Comment 77]
*** Bug 211429 has been marked as a duplicate of this bug. ***
Blocks: 228464
Depends on: 214063
Summary: Compiler warnings → Fix compiler warnings: "unused variable" and "defined but not used"
No longer blocks: 228464
Blocks: 187528
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]


'approval1.6=?': Trivial unused code removal.
(if not too late)
Attachment #130280 - Flags: approval1.6?
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]


'approval1.6=?': Trivial unused code removal.
(if not too late)

Note: even if it's not in the "Blamed Build Warnings" page :-<
Attachment #130296 - Flags: approval1.6?
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]


This patch is out of sync.: needs an update.

Note: even if it's not in the "Blamed Build Warnings" page :-<
Comment on attachment 130305 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v1


This patch is out of sync.: needs an update.

Note: even if it's not in the "Blamed Build Warnings" page :-<
Comment on attachment 130314 [details] [diff] [review]
Patch for nsBoxObject.cpp, v2


This patch was included in attachment 131865 [details] [diff] [review] of bug 219908 :-)
Attachment #130314 - Attachment is obsolete: true
Comment on attachment 130328 [details] [diff] [review]
Patch for nsXMLContentSink.cpp, v2


This was fixed in v1.288,
apparently from bug 215981 :-)
Attachment #130328 - Attachment is obsolete: true
Comment on attachment 130353 [details] [diff] [review]
Patch for nsDocumentViewer.cpp, v2


Superseeded by attachment 134374 [details] [diff] [review] in bug 224018 :-)
Attachment #130353 - Attachment is obsolete: true
Depends on: 228780
Depends on: 90906
Depends on: 224018
Depends on: 214199

Comment 97

14 years ago
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]

Too late in the 1.6 timeframe for changes that aren't serious
regressions, security, or crash related.

Comment 98

14 years ago
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]

Too late in the 1.6 timeframe for changes that aren't serious
regressions, security, or crash related.
Attachment #130296 - Flags: approval1.6? → approval1.6-

Updated

14 years ago
Attachment #130280 - Flags: approval1.6? → approval1.6-
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]


matthias (or anyone):
Can you check in your r+/sr+ patch ? Thanks.
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]


Matthias (or anyone):
Can you check in your r+/sr+ patch ? Thanks.
Re comment 55:

MXR search for "NS_ASSERTION\(printSettings":

timeless:
Would you like me to prepare a patch to replace theses 2
{
/gfx/src/nsPrintOptionsImpl.cpp, line 859 -- NS_ASSERTION(printSettings, "Can't
be NULL!");
/gfx/src/windows/nsPrintOptionsWin.cpp, line 68 -- NS_ASSERTION(printSettings,
"Can't be NULL!");
}
by
|if (!printSettings) return NS_ERROR_OUT_OF_MEMORY;|
?

There are theses 4 also, but they are not returning a |nsresult|, then I don't know
{
/embedding/qa/mozembed/src/mozEmbed.cpp, line 663 -- NS_ASSERTION(printSettings,
"You can't PrintPreview without a PrintSettings!");
/embedding/tests/os2Embed/os2Embed.cpp, line 673 -- NS_ASSERTION(printSettings,
"You can't PrintPreview without a PrintSettings!");
/embedding/tests/winEmbed/winEmbed.cpp, line 624 -- NS_ASSERTION(printSettings,
"You can't PrintPreview without a PrintSettings!");
/content/base/src/nsDocumentViewer.cpp, line 2152 -- NS_ASSERTION(printSettings,
"You can't PrintPreview without a PrintSettings!");
}
Created attachment 137772 [details] [diff] [review]
<nsScreenManagerWin.*> (v6)
[+/- Checked in: Comment 110]

This is v5, with comment 85 suggestions.
Attachment #131244 - Attachment is obsolete: true
Comment on attachment 137772 [details] [diff] [review]
<nsScreenManagerWin.*> (v6)
[+/- Checked in: Comment 110]


'r=?':
I have no compiler: Could you compile/test/review it ? Thanks.


The only call to |nsScreenManagerWin::GetNumberOfScreens()| is at
<http://landfill.bugzilla.org/mxr-test/seamonkey/source/gfx/src/windows/nsDevic
eContextWin.cpp#189>.
Should something be done there too ? (What ?)
Attachment #137772 - Flags: review?(ere)

Comment 104

14 years ago
Comment on attachment 137772 [details] [diff] [review]
<nsScreenManagerWin.*> (v6)
[+/- Checked in: Comment 110]

I don't have MinGW so I'll just rely on them being correct. Otherwise ok. r=ere
Attachment #137772 - Flags: review?(ere) → review+
Attachment #137772 - Flags: superreview?(rbs)

Comment 105

14 years ago
Comment on attachment 137772 [details] [diff] [review]
<nsScreenManagerWin.*> (v6)
[+/- Checked in: Comment 110]

sr=rbs
Attachment #137772 - Flags: superreview?(rbs) → superreview+
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]

Re comment 46:

timeless:
What do you mean by "this change should be made to both windows and os2
plugins." ?

MXR result:
{
szDefaultPluginFinderURL
Defined as a variable in:

    * modules/plugin/samples/default/os2/plugin.cpp, line 54
    * modules/plugin/samples/default/windows/dialogs.cpp, line 47
    * modules/plugin/samples/default/windows/plugin.cpp, line 57 

Referenced (in 3 files total) in:

    * modules/plugin/samples/default/os2/plugin.cpp:
	  o View change log or Blame annotations line 54
	  o line 324 
    * modules/plugin/samples/default/windows/dialogs.cpp:
	  o View change log or Blame annotations line 47 
    * modules/plugin/samples/default/windows/plugin.cpp:
	  o View change log or Blame annotations line 57
	  o line 369 
}
Comment on attachment 130332 [details] [diff] [review]
<os2/dialogs.cpp>
[Checked in: Comment 107]


Check in: { 1.5 timeless%mozdev.org	Sep 10 20:18 }
Attachment #130332 - Attachment description: Patch for dialogs.cpp (os2) → <os2/dialogs.cpp> [Checked in: Comment 107]
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]

Disregard comment 106:

I read "dialogs.cpp" where it is written "plugin.cpp" :-<
(I believe the 2 plugin.cpp files are correct as they are.)
Attachment #130274 - Attachment description: Patch for dialogs.cpp (win) → <windows/dialogs.cpp>
Updating:
+(CC) brendan

I'm changing r?/sr? from beard#netscape.com to you,
since the owner and his 3 peers have @netscape.com email in <owners.html>,
and I don't know to who/what this 'modules/plugin' fits in <reviewers.html>.

Can you help to find a (super-)reviewer ?

Thanks.
Attachment #130274 - Flags: superreview?(beard) → superreview?(brendan)
Attachment #130295 - Attachment description: Patch for nsPluginsDirWin.cpp → <nsPluginsDirWin.cpp>
Attachment #130295 - Flags: superreview?(beard) → superreview?(brendan)
Attachment #130329 - Attachment description: Patch for plugin.cpp, v2 → <windows/plugin.cpp> v2
Attachment #130329 - Flags: superreview?(brendan)
Attachment #130329 - Flags: superreview?(beard)
Attachment #130329 - Flags: review?(brendan)
Attachment #130329 - Flags: review?(beard)
Comment on attachment 137772 [details] [diff] [review]
<nsScreenManagerWin.*> (v6)
[+/- Checked in: Comment 110]


Check in: { 12/24/2003 15:45	neil%parkwaycc.co.uk	nsScreenManagerWin.cpp 
1.16 }
"I only checked in nsScreenManagerWin.cpp because nsScreenManagerWin.h change
gave me an *extra* warning..."
Attachment #137772 - Attachment description: <nsScreenManagerWin.*> (v6) → <nsScreenManagerWin.*> (v6) [+/- Checked in: Comment 110]
Attachment #137772 - Attachment is obsolete: true

Comment 111

14 years ago
*** Bug 198785 has been marked as a duplicate of this bug. ***
Comment on attachment 130297 [details] [diff] [review]
Patch for nsRDFXMLDataSource.cpp
[Checked in: Comment 112]


Check in: { 1.145	timeless%mozdev.org	Sep 10 2003 }
Attachment #130297 - Attachment description: Patch for nsRDFXMLDataSource.cpp → Patch for nsRDFXMLDataSource.cpp [Checked in: Comment 112]
Comment on attachment 130280 [details] [diff] [review]
</widget/src/windows/nsClipboard.cpp>
[Checked in: Comment 113]

Checking in nsClipboard.cpp;
/cvsroot/mozilla/widget/src/windows/nsClipboard.cpp,v  <--  nsClipboard.cpp
new revision: 1.86; previous revision: 1.85

Next time, please for the love of God use cvs diff or at least diff from
toplevel -- there are 8 different nsClipboard.cpp files in the tree, and _how_
do you expect people to figure out which one to apply your patch to?
Attachment #130280 - Attachment is obsolete: true
Attachment #130280 - Attachment description: Patch for nsClipboard.cpp → Patch for nsClipboard.cpp [Checked in: Comment 113]
Attachment #130280 - Attachment description: Patch for nsClipboard.cpp [Checked in: Comment 113] → </widget/src/windows/nsClipboard.cpp> [Checked in: Comment 113]
Attachment #130281 - Attachment description: Patch for nsDataObj.cpp → </widget/src/windows/nsDataObj.cpp>
Attachment #130329 - Attachment description: <windows/plugin.cpp> v2 → </modules/plugin/samples/default/windows/plugin.cpp> v2
Attachment #130306 - Attachment description: Patch for plugin.cpp → </modules/plugin/samples/default/windows/plugin.cpp> v1
Attachment #130327 - Attachment description: Patch for jvmmgr.cpp, v2 → </modules/oji/src/jvmmgr.cpp> v2
Attachment #130275 - Attachment description: Patch for jvmmgr.cpp → </modules/oji/src/jvmmgr.cpp> v1
Attachment #130305 - Attachment description: Patch for nsXULTooltipListener.cpp → </layout/xul/base/src/nsXULTooltipListener.cpp>
Attachment #130302 - Attachment description: Patch for nsWinCharset.cpp → </intl/uconv/src/nsWinCharset.cpp>
Attachment #130296 - Attachment description: Patch for nsPrintOptionsWin.cpp → </gfx/src/windows/nsPrintOptionsWin.cpp>
Attachment #130295 - Attachment description: <nsPluginsDirWin.cpp> → </modules/plugin/base/src/nsPluginsDirWin.cpp>
Attachment #130292 - Attachment description: Patch for nsNativeThemeWin.cpp → </gfx/src/windows/nsNativeThemeWin.cpp>
Attachment #130291 - Attachment description: Patch for nsNativeDragTarget.cpp → </widget/src/windows/nsNativeDragTarget.cpp>
Attachment #130287 - Attachment description: Patch for nsGfxFactoryWin.cpp → </gfx/src/windows/nsGfxFactoryWin.cpp>
Attachment #130274 - Attachment description: <windows/dialogs.cpp> → </modules/plugin/samples/default/windows/dialogs.cpp>
Depends on: 219908
Comment on attachment 130296 [details] [diff] [review]
</gfx/src/windows/nsPrintOptionsWin.cpp>
[Checked in: Comment 114]

Checking in nsPrintOptionsWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsPrintOptionsWin.cpp,v  <-- 
nsPrintOptionsWin.cpp
new revision: 1.8; previous revision: 1.7
Attachment #130296 - Attachment is obsolete: true
Attachment #130296 - Attachment description: </gfx/src/windows/nsPrintOptionsWin.cpp> → </gfx/src/windows/nsPrintOptionsWin.cpp> [Checked in: Comment 114]
Comment on attachment 130305 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v1


This is what I asked jag
{
> I guess the warning used to be "defined but not used", but this is no longer the case !!?
> See
> http://bugzilla.mozilla.org/show_bug.cgi?id=219908
> http://bugzilla.mozilla.org/attachment.cgi?id=131865&action=view
> with "fixed" the warning (which appeared in v1.23) in v1.28
}

And his answer was
{ (1/2)
I think I'd rather keep it in there
}
Attachment #130305 - Attachment description: </layout/xul/base/src/nsXULTooltipListener.cpp> → </layout/xul/base/src/nsXULTooltipListener.cpp>, v1
Attachment #130305 - Attachment is obsolete: true
Created attachment 139448 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v2
[Checked in: Comment 119]

{ (2/2)
perhaps adding a comment saying "this stuff inside DEBUG_crap could be used to
make tree tooltips work in the future."
}
Comment on attachment 139448 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v2
[Checked in: Comment 119]

'(s)r=?': (see comment 116)
I have no compiler: Could you compile/test/review it ? Thanks.
Attachment #139448 - Flags: superreview?(jag)
Attachment #139448 - Flags: review?(bryner)

Comment 118

14 years ago
Comment on attachment 139448 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v2
[Checked in: Comment 119]

sr=jag
Attachment #139448 - Flags: superreview?(jag) → superreview+
Attachment #139448 - Flags: review?(bryner) → review+
Comment on attachment 139448 [details] [diff] [review]
</layout/xul/base/src/nsXULTooltipListener.cpp>, v2
[Checked in: Comment 119]


Check in: { 01/22/2004 08:14	neil%parkwaycc.co.uk	mozilla/ layout/ xul/
base/ src/ nsXULTooltipListener.cpp	  1.32 }
Attachment #139448 - Attachment description: </layout/xul/base/src/nsXULTooltipListener.cpp>, v2 → </layout/xul/base/src/nsXULTooltipListener.cpp>, v2 [Checked in: Comment 119]
Attachment #139448 - Attachment is obsolete: true
Comment on attachment 130287 [details] [diff] [review]
</gfx/src/windows/nsGfxFactoryWin.cpp>


No (super-)review from <hyatt@mozilla.org> since '2003-08-24' :-(

Also, see comment 48 !
Attachment #130287 - Flags: superreview?(rbs)
Attachment #130287 - Flags: superreview?(hyatt)
Attachment #130287 - Flags: review?(hyatt)
Attachment #130287 - Flags: review?(ere)
Comment on attachment 130292 [details] [diff] [review]
</gfx/src/windows/nsNativeThemeWin.cpp>
[Checked in: Comment 135]


No review from <tim@prismelite.com>, no super-review from <hyatt@mozilla.org>,
since '2003-08-24' :-(

Also, see comment 53 !
Attachment #130292 - Flags: superreview?(rbs)
Attachment #130292 - Flags: superreview?(hyatt)
Attachment #130292 - Flags: review?(tim)
Attachment #130292 - Flags: review?(ere)
Comment on attachment 130281 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1


No (super-)review from <blizzard@mozilla.org> since '2003-08-23' :-(
Attachment #130281 - Flags: superreview?(bryner)
Attachment #130281 - Flags: superreview?(blizzard)
Attachment #130281 - Flags: review?(ere)
Attachment #130281 - Flags: review?(blizzard)
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]


No super-review from <blizzard@mozilla.org> since '2003-08-23' :-(

Also, see comment 92 !
(If sr+, I'll post an updated patch for check-in.)
Attachment #130302 - Flags: superreview?(blizzard) → superreview?(dveditz)
Comment on attachment 130327 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v2
[Checked in: Comment 166]


No (super-)review from <scc@mozilla.org> since '2003-08-24' :-(
Attachment #130327 - Flags: superreview?(scc)
Attachment #130327 - Flags: superreview?(dveditz)
Attachment #130327 - Flags: review?(scc)
Attachment #130327 - Flags: review?(kyle.yuan)
Comment on attachment 130329 [details] [diff] [review]
<plugin.cpp>, v2

No (super-)review from <brendan@mozilla.org> since '2003-12-23' :-(
Attachment #130329 - Flags: superreview?(brendan)
Attachment #130329 - Flags: superreview?(alecf)
Attachment #130329 - Flags: review?(peterlubczynski-bugs)
Attachment #130329 - Flags: review?(brendan)
Comment on attachment 130295 [details] [diff] [review]
</modules/plugin/base/src/nsPluginsDirWin.cpp>
[Checked in: Comment 155]


No super-review from <brendan@mozilla.org> since '2003-12-23' :-(
Attachment #130295 - Flags: superreview?(brendan) → superreview?(alecf)
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]


No super-review from <brendan@mozilla.org> since '2003-12-23' :-(

Also, see comment 108 !
Attachment #130274 - Flags: superreview?(brendan) → superreview?(alecf)
Serge, if you are removing unused code, and the module owner or peer stamps his
or her r= on your patch, and someone tests it, then you can assume
rs=brendan@mozilla.org in lieu of sr=.

But attachment 130329 [details] [diff] [review] gets rid of set-once-never-used variables, when I would
rather see a change to check for errors, if errors are possible (they seem to be
in this code).  Don't just paper over warnings; if you are going to touch the
file, you may as well improve it substantively.

/be
(In reply to comment #128)
> Serge, if you are removing unused code, and the module owner or peer stamps his
> or her r= on your patch, and someone tests it, then you can assume
> rs=brendan@mozilla.org in lieu of sr=.

I'm not familiar with |rs=| ... I guess it means that no super-review is needed !?

> But attachment 130329 [details] [diff] [review] gets rid of set-once-never-used variables, when I would
> rather see a change to check for errors, if errors are possible (they seem to be
> in this code).  Don't just paper over warnings; if you are going to touch the
> file, you may as well improve it substantively.

I agree, and do it as much as I can;
In these cases, the patches are not mine (untill I would take and modify them);
I'm only trying to get them dealt with, as a first step:
depending on the reviewer comment, I might either update them myself, or ask for
 helpwanted...


Matthias Bockelkamp:
Can/Will you check in the patches which should be (current / future) ?
Or should I do it "myself" ? (Let's say this is the default answer after the
next 7 days...)
Serge: rs= means "rubber-stamp=", amusingly inverting the sr= "super-review="
letter order.  Don't abuse it -- use it only with module owner or peer review,
only with testing, and only for changes that remove dead code.

In that light, you have rs= on attachment 130329 [details] [diff] [review] as well, but I'd appreciate it
if you could try to get a followup fix to check for errors done.  If that's
beyond what you're willing or able to do, please help find an owner or peer, or
a new peer, who can help.  (Any patch that adds error checks would need real
sr=, as usual.)  Thanks,

/be

Updated

13 years ago
Attachment #130327 - Flags: review?(kyle.yuan) → review+

Comment 131

13 years ago
Comment on attachment 130287 [details] [diff] [review]
</gfx/src/windows/nsGfxFactoryWin.cpp>

r+sr=rbs
Attachment #130287 - Flags: superreview?(rbs)
Attachment #130287 - Flags: superreview+
Attachment #130287 - Flags: review?(ere)
Attachment #130287 - Flags: review+

Comment 132

13 years ago
Comment on attachment 130292 [details] [diff] [review]
</gfx/src/windows/nsNativeThemeWin.cpp>
[Checked in: Comment 135]

r+sr=rbs
Attachment #130292 - Flags: superreview?(rbs)
Attachment #130292 - Flags: superreview+
Attachment #130292 - Flags: review?(ere)
Attachment #130292 - Flags: review+
(Reporter)

Comment 133

13 years ago
(In reply to comment #129)
> Can/Will you check in the patches which should be (current / future) ?
> Or should I do it "myself" ? (Let's say this is the default answer after the
> next 7 days...)

I cannot check anything in. If you have the necessary rights, please check it in.

(In reply to comment #130)
> In that light, you have rs= on attachment 130329 [details] [diff] [review] as well, but I'd appreciate it
> if you could try to get a followup fix to check for errors done.  If that's

I thought about something like:

int iSize = LoadString(m_hInst, IDS_GOING2HTML, buf, sizeof(buf));

NPError error = NPN_NewStream(m_pNPInstance, "text/html", "asd_plugin_finder",
&pStream);

if (error == NPERR_NO_ERROR) {

  //char buf[] = "<html>\n<body>\n\n<h2 align=center>NPN_NewStream / NPN_Write -
This seems to work.</h2>\n\n</body>\n</html>";

  int32 iBytes = NPN_Write(m_pNPInstance, pStream, iSize, buf);  // iSize was
lstrlen(buf) before!!!
  
  NPN_DestroyStream(m_pNPInstance, pStream, NPRES_DONE);
  
}  

Two questions, since I'm not familar with the plugin code:
- What should happen if error!=NPERR_NO_ERROR?
- What should happen if after NPN_Write iSize!=iBytes?
- Should CPlugin::URLNotify perhaps be changed from void to NPError?
Attachment #130327 - Flags: superreview?(dveditz)
(In reply to comment #131)
> (From update of attachment 130287 [details] [diff] [review])
> r+sr=rbs

I'd like to have your comment on comment 48 before proceeding with check-in.
Thanks.
Comment on attachment 130292 [details] [diff] [review]
</gfx/src/windows/nsNativeThemeWin.cpp>
[Checked in: Comment 135]


Check in: { 2004-04-21 08:34	neil%parkwaycc.co.uk	mozilla/ gfx/ src/
windows/ nsNativeThemeWin.cpp	     3.38    0/8     Removing unused variables
b=130292 p=mbockelkamp@web.de r/sr=rbs }
Attachment #130292 - Attachment description: </gfx/src/windows/nsNativeThemeWin.cpp> → </gfx/src/windows/nsNativeThemeWin.cpp> [Checked in: Comment 135]
Attachment #130292 - Attachment is obsolete: true
Comment on attachment 130327 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v2
[Checked in: Comment 166]


Resurrect sr=?:
I have no compiler, neil (my check-in partner) has no jvm;
Or can someone test this ? (kyle ?)
Attachment #130327 - Flags: superreview?(dveditz)

Comment 137

13 years ago
> I'd like to have your comment on comment 48

I also prefer using the generic constructor, unless some special-casing prevents
otherwise. See nsGfxFactoryGTK/Mac. I hadn't see that comment and assumed that
you guys did your homework properly while waiting all this time. Don't just toss
review requests in the air. Seeing '-' lines is tempting.

Comment 138

13 years ago
Comment on attachment 130281 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1

r=ere
Attachment #130281 - Flags: review?(ere) → review+
Attachment #130281 - Attachment description: </widget/src/windows/nsDataObj.cpp> → </widget/src/windows/nsDataObj.cpp>, v1
Attachment #130281 - Attachment is obsolete: true
Attachment #130281 - Flags: superreview?(bryner)
Created attachment 146811 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1b
[Checked in: Comment 142]

v1,
with an additional |status| removal.
Comment on attachment 146811 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1b
[Checked in: Comment 142]


I have no compiler: Could you compile/test/review this patch ? Thanks.

Sorry for asking a second time:
the first patch was not mine; (and may be MinGW did not complain)
Now, I ran an LXR search on |GlobalUnlock| :-)
Attachment #146811 - Flags: review?(ere)

Comment 141

13 years ago
Comment on attachment 146811 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1b
[Checked in: Comment 142]

Compiles, works, r=ere
Attachment #146811 - Flags: review?(ere) → review+
Comment on attachment 146811 [details] [diff] [review]
</widget/src/windows/nsDataObj.cpp>, v1b
[Checked in: Comment 142]


Check in: { 2004-04-23 02:01	neil%parkwaycc.co.uk	mozilla/ widget/ src/
windows/ nsDataObj.cpp	  1.61 }
Attachment #146811 - Attachment description: </widget/src/windows/nsDataObj.cpp>, v1b → </widget/src/windows/nsDataObj.cpp>, v1b [Checked in: Comment 142]
Attachment #146811 - Attachment is obsolete: true
Created attachment 146892 [details] [diff] [review]
<nsFrame.cpp>
[Checked in: Comment 147]

Fix for:

{{ <http://tinderbox.mozilla.org/SeaMonkey/warn1082753340.30566.html>
1173.	layout/html/base/src/nsFrame.cpp:4352 (See build log excerpt)
	`nsIFrame* GetNextSiblingAcrossLines(nsIPresContext*, nsIFrame*)'
defined but not used
}}

{{ <http://lxr.mozilla.org/mozilla/search?string=GetNextSiblingAcrossLines>
GetNextSiblingAcrossLines
/layout/html/base/src/nsFrame.cpp, line 4352 --
GetNextSiblingAcrossLines(nsIPresContext *aPresContext, nsIFrame *aFrame)
}}
Comment on attachment 146892 [details] [diff] [review]
<nsFrame.cpp>
[Checked in: Comment 147]


(In reply to comment #142)
> (From update of Comment 142]">attachment 146811 [details] [diff] [review])

It removes a useless |thisBlock| too.

***

I have no compiler: Could you (super-)review/compile/test/check in this patch ?
Thanks.
Attachment #146892 - Flags: superreview?(dbaron)
Attachment #146892 - Flags: review?(dbaron)
Created attachment 146894 [details] [diff] [review]
<nsMsgThread.cpp>, v1

Fix for:

{{ <http://tinderbox.mozilla.org/SeaMonkey/warn1082753340.30566.html>
4.	mailnews/db/msgdb/src/nsMsgThread.cpp:871 (See build log excerpt)
	`nsresult nsMsgThreadUnreadFilter(nsIMsgDBHdr*, void*)' defined but not
used
}}

{{ <http://lxr.mozilla.org/mozilla/search?string=nsMsgThreadUnreadFilter>
nsMsgThreadUnreadFilter
/mailnews/db/msgdb/src/nsMsgThread.cpp, line 870 --
nsMsgThreadUnreadFilter(nsIMsgDBHdr* msg, void* closure)
}}

And some cleanup, mostly related to |ret|.
Comment on attachment 146894 [details] [diff] [review]
<nsMsgThread.cpp>, v1


I have no compiler: Could you (super-)review/compile/test/check in this patch ?
Thanks.
Attachment #146894 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #146894 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #146892 - Flags: superreview?(dbaron)
Attachment #146892 - Flags: superreview+
Attachment #146892 - Flags: review?(dbaron)
Attachment #146892 - Flags: review+
Comment on attachment 146892 [details] [diff] [review]
<nsFrame.cpp>
[Checked in: Comment 147]


Check in: { 2004-04-24 06:30	neil%parkwaycc.co.uk	mozilla/ layout/ html/
base/ src/ nsFrame.cpp	 3.481 }
Attachment #146892 - Attachment description: <nsFrame.cpp> → <nsFrame.cpp> [Checked in: Comment 147]
Attachment #146892 - Attachment is obsolete: true
Comment on attachment 146894 [details] [diff] [review]
<nsMsgThread.cpp>, v1

>-  ret = m_mdbDB->UInt32ToRowCellColumn(m_metaRow, m_mdbDB->m_threadIdColumnToken, threadKey);
>+  nsresult ret = m_mdbDB->UInt32ToRowCellColumn(
>+                   m_metaRow, m_mdbDB->m_threadIdColumnToken, threadKey);
>   // gotta set column in meta row here.
>   return ret;
Thought: return m_mdbDB->UInt32ToRowCellColumn( [etc] here (and similar cases
below)?

>-  if (ret == 0)
>+  if (ret == NS_OK)
Still wrong, should be if (NS_SUCCEEDED(ret))

>-  nsresult ret = NS_OK;
If ret gets assigned into so that the NS_OK is never used, then just declaring
nsresult ret; should be sufficient cleanup.

>-  ret = m_mdbTable->GetTableRowCursor(m_mdbDB->GetEnv(), pos, &rowCursor);
>-  if (ret != 0)
>+  nsresult ret = m_mdbTable->GetTableRowCursor(
>+                               m_mdbDB->GetEnv(), pos, &rowCursor);
>+  if (ret != NS_OK)
>     return  NS_ERROR_FAILURE;
This should actually say
if (NS_FAILED(ret))
  return ret;
Attachment #146894 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #146894 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #146894 - Flags: review-

Comment 149

13 years ago
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]

sr=alecf
Attachment #130274 - Flags: superreview?(alecf) → superreview+

Comment 150

13 years ago
Comment on attachment 130295 [details] [diff] [review]
</modules/plugin/base/src/nsPluginsDirWin.cpp>
[Checked in: Comment 155]

sr=alecf
Attachment #130295 - Flags: superreview?(alecf) → superreview+

Comment 151

13 years ago
Comment on attachment 130329 [details] [diff] [review]
<plugin.cpp>, v2

huh. I'm wondering if this is really the right thing. I wonder if iSize should
be passed in instead of lstrlen(buf) and if "rc" should be checked before the
call to NPN_Write and NPN_Destroy, instead of just throwing away the return
values.

cancelling my sr request until we get proper reviews from the module owner.
Attachment #130329 - Flags: superreview?(alecf)
(Reporter)

Comment 152

13 years ago
Gerv:
Please have a look at Attachment 130329 [details] [diff] and Comments 113, 151.
I cannot find the email address of the initial developer (av@netscape.com). 
Comment on attachment 130274 [details] [diff] [review]
</modules/plugin/samples/default/windows/dialogs.cpp>
[Checked in: Comment 153]


Check in: { 2004-04-29 15:58	neil%parkwaycc.co.uk	mozilla/ modules/
plugin/ samples/ default/ windows/ dialogs.cpp	      1.9 }
Attachment #130274 - Attachment description: </modules/plugin/samples/default/windows/dialogs.cpp> → </modules/plugin/samples/default/windows/dialogs.cpp> [Checked in: Comment 153]
Attachment #130274 - Attachment is obsolete: true
(In reply to comment #46)
> (From update of Comment 153]">attachment 130274 [details] [diff] [review])
> this change should be made to both windows and os2 plugins.

I filed bug 242215 on this.
Comment on attachment 130295 [details] [diff] [review]
</modules/plugin/base/src/nsPluginsDirWin.cpp>
[Checked in: Comment 155]


Check in: { 2004-04-30 11:28	neil%parkwaycc.co.uk	mozilla/ modules/
plugin/ base/ src/ nsPluginsDirWin.cpp	      1.34 }
Attachment #130295 - Attachment description: </modules/plugin/base/src/nsPluginsDirWin.cpp> → </modules/plugin/base/src/nsPluginsDirWin.cpp> [Checked in: Comment 155]
Attachment #130295 - Attachment is obsolete: true
Attachment #130329 - Attachment description: </modules/plugin/samples/default/windows/plugin.cpp> v2 → </modules/plugin/samples/default/windows/plugin.cpp> , v2
Attachment #130329 - Attachment is obsolete: true
Attachment #130329 - Flags: review?(peterlubczynski-bugs)
Created attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

v2, with comment 151 suggestion(s).
Comment on attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

I have no compiler: Could you compile/test/review this patch ? Thanks.

(In reply to comment #152)
> Please have a look at Attachment 130329 [details] [diff] and Comments 113.

See also comment 133 (not 113).
Attachment #147420 - Flags: review?(peterlubczynski-bugs)
Attachment #146894 - Attachment is obsolete: true
Created attachment 147444 [details] [diff] [review]
<nsMsgThread.cpp>, v2
[Checked in: Comment 161]

v1, with comment 148 suggestion(s).
Comment on attachment 147444 [details] [diff] [review]
<nsMsgThread.cpp>, v2
[Checked in: Comment 161]

I have no compiler: Could you (super-)review/compile/test/check in this patch ?
Thanks.
Attachment #147444 - Flags: superreview?(bienvenu)
Attachment #147444 - Flags: review?(bienvenu)
Matthias: what do you want me to look at about that attachment? I can't work it out.

Gerv

Comment 161

13 years ago
Comment on attachment 147444 [details] [diff] [review]
<nsMsgThread.cpp>, v2
[Checked in: Comment 161]

r/sr=bienvenu, checked in.
Attachment #147444 - Attachment is obsolete: true
Attachment #147444 - Flags: review?(bienvenu)
Attachment #147444 - Flags: review+
Attachment #147444 - Flags: approval1.7+

Comment 162

13 years ago
Comment on attachment 147444 [details] [diff] [review]
<nsMsgThread.cpp>, v2
[Checked in: Comment 161]

whoa, didn't mean to grant 1.7 approval (don't think I did either...) running
trun seamonkey build...
Attachment #147444 - Flags: superreview?(bienvenu)
Attachment #147444 - Flags: superreview+
Attachment #147444 - Flags: approval1.7+
(Reporter)

Comment 163

13 years ago
(In reply to comment #160)
> Matthias: what do you want me to look at about that attachment? I can't work
it out.

The reviewer wrote that the rc value should not be removed but used for an error
check. Serge has already done that in attachment 147420 [details] [diff] [review].
Attachment #147444 - Attachment description: <nsMsgThread.cpp>, v2 → <nsMsgThread.cpp>, v2 [Checked in: Comment 161]
(In reply to comment #136)
> (From update of attachment 130327 [details] [diff] [review])
> 
> Resurrect sr=?:
> I have no compiler, neil (my check-in partner) has no jvm;
> Or can someone test this ? (kyle ?)

Kyle ? We're looking for a tester or a super-reviewer ... Can you help ? Thanks.

Updated

13 years ago
Attachment #130327 - Flags: superreview?(dveditz) → superreview?(Henry.Jia)

Comment 165

13 years ago
Comment on attachment 130327 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v2
[Checked in: Comment 166]

sr=Henry
Attachment #130327 - Flags: superreview?(Henry.Jia) → superreview+
Comment on attachment 130327 [details] [diff] [review]
</modules/oji/src/jvmmgr.cpp> v2
[Checked in: Comment 166]


Check in: { 2004-05-25 03:34	neil%parkwaycc.co.uk	mozilla/ modules/ oji/
src/ jvmmgr.cpp  1.44 }
Attachment #130327 - Attachment description: </modules/oji/src/jvmmgr.cpp> v2 → </modules/oji/src/jvmmgr.cpp> v2 [Checked in: Comment 166]
Attachment #130327 - Attachment is obsolete: true
Comment on attachment 130291 [details] [diff] [review]
</widget/src/windows/nsNativeDragTarget.cpp> [Checked in: Comment 173]

No super-review from <hyatt@mozilla.org> since "2003-08-24" :-(
Attachment #130291 - Flags: superreview?(hyatt) → superreview?(rbs)
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]

No super-review from <dveditz@cruzio.com> since "2004-04-20" :-(
Attachment #130302 - Flags: superreview?(dveditz) → superreview?(jag)
Comment on attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

No review from <peterlubczynski-bugs@peterl.com> since "2004-04-30" :-(

I have no compiler: Could you compile/test/(super-)review/check in this patch ?
Thanks.

***

(In reply to comment #152)
> Please have a look at Attachment 130329 [details] [diff] and Comments 113.

See also comment 133 (not 113).
Attachment #147420 - Flags: superreview?(alecf)
Attachment #147420 - Flags: review?(peterlubczynski-bugs)
Attachment #147420 - Flags: review?(alecf)

Updated

13 years ago
Attachment #130287 - Attachment is obsolete: true
Attachment #130287 - Flags: superreview-
Attachment #130287 - Flags: superreview+
Attachment #130287 - Flags: review-
Attachment #130287 - Flags: review+

Updated

13 years ago
Attachment #130291 - Flags: superreview?(rbs) → superreview+

Comment 170

13 years ago
Comment on attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

I will review this patch when it has been tested.
Attachment #147420 - Flags: superreview?(alecf)
Attachment #147420 - Flags: review?(alecf)
(In reply to comment #170)
> (From update of attachment 147420 [details] [diff] [review])
> I will review this patch when it has been tested. 

helpwanted:
Who can test this patch ? (Matthias ?)
(Reporter)

Comment 172

13 years ago
Comment on attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

(In reply to comment #171)
> Who can test this patch ? (Matthias ?)

Compiles (with mingw) and works (with Shockwave Flash).
Attachment #147420 - Flags: superreview?(alecf)
Attachment #147420 - Flags: review?(alecf)
(Reporter)

Comment 173

13 years ago
Comment on attachment 130291 [details] [diff] [review]
</widget/src/windows/nsNativeDragTarget.cpp> [Checked in: Comment 173]

Check in: { 2004-05-26 04:48	neil%parkwaycc.co.uk	mozilla/ widget/ src/
windows/ nsNativeDragTarget.cpp   1.28	  3/3	  Removing unused variable
b=217089 p=mbockelcamp@web.de r=timeless sr=rbs }
Attachment #130291 - Attachment description: </widget/src/windows/nsNativeDragTarget.cpp> → </widget/src/windows/nsNativeDragTarget.cpp> [Checked in: Comment 173]
Attachment #130291 - Attachment is obsolete: true

Comment 174

13 years ago
Comment on attachment 130302 [details] [diff] [review]
<nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]

sr=jag
Attachment #130302 - Flags: superreview?(jag) → superreview+
Attachment #130302 - Attachment description: </intl/uconv/src/nsWinCharset.cpp> → </intl/uconv/src/nsWinCharset.cpp> (-v1, for review only)
Attachment #130302 - Attachment is obsolete: true
Attachment #130302 - Attachment description: </intl/uconv/src/nsWinCharset.cpp> (-v1, for review only) → <nsWinCharset.cpp>, v1-r (for review only) [Check in: See v1-ci]
Created attachment 149385 [details] [diff] [review]
<nsWinCharset.cpp>, v1-ci (for check in only)
[Checked in: Comment 177]

v1-r, "diffed" against current trunk,
with an added white space cleanup.

Keeping
{{
<nsWinCharset.cpp>, v1-r (for review only)	 patch		2003-08-23
12:27 PDT    smontagu: review+
jag: superreview+ 
}}

Comment 176

13 years ago
Comment on attachment 147420 [details] [diff] [review]
<*/plugin.cpp> ++, v3

thanks. 

comments:
+// static char szDefaultFileExt[] = "*";

remove this line entirely, don't just comment it out, goes for both instances

r/sr=alecf with that fixed
Comment on attachment 149385 [details] [diff] [review]
<nsWinCharset.cpp>, v1-ci (for check in only)
[Checked in: Comment 177]


Check in: { 2004-05-27 02:00	neil%parkwaycc.co.uk	mozilla/ intl/ uconv/
src/ nsWinCharset.cpp	  1.33 }
Attachment #149385 - Attachment description: <nsWinCharset.cpp>, v1-ci (for check in only) → <nsWinCharset.cpp>, v1-ci (for check in only) [Checked in: Comment 177]
Attachment #149385 - Attachment is obsolete: true
Attachment #130329 - Attachment description: </modules/plugin/samples/default/windows/plugin.cpp> , v2 → <plugin.cpp>, v2
Attachment #147420 - Attachment is obsolete: true
Attachment #147420 - Flags: superreview?(alecf)
Attachment #147420 - Flags: review?(alecf)
Created attachment 149425 [details] [diff] [review]
<*/plugin.cpp> ++, v3b
[Checked in: Comment 180]

v3b, with comment 176 suggestion(s).
Comment on attachment 149425 [details] [diff] [review]
<*/plugin.cpp> ++, v3b
[Checked in: Comment 180]

Keeping
{{
(comment #176)
> 
> r/sr=alecf
}}
Attachment #149425 - Flags: superreview+
Attachment #149425 - Flags: review+
Comment on attachment 149425 [details] [diff] [review]
<*/plugin.cpp> ++, v3b
[Checked in: Comment 180]


Check in: { 2004-05-27 12:32	neil%parkwaycc.co.uk }
Attachment #149425 - Attachment description: <*/plugin.cpp> ++, v3b → <*/plugin.cpp> ++, v3b [Checked in: Comment 180]
Attachment #149425 - Attachment is obsolete: true
Product: Browser → Seamonkey
Comment on attachment 130287 [details] [diff] [review]
</gfx/src/windows/nsGfxFactoryWin.cpp>

(In reply to comment #137)
> I also prefer using the generic constructor

Eventually, this file was removed by
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/gfx/src/windows/Attic/nsGfxFactoryWin.cpp&rev=3.46

Comment 182

9 years ago
Serge, can this bug be closed?

Updated

7 years ago
Assignee: mbockelkamp → nobody
Component: General → General
Product: SeaMonkey → Core
QA Contact: general → general
Whiteboard: bugday0420
Whiteboard: bugday0420 → [build_warning] bugday0420
All patches checked in, marking fixed.
Rest of the work is carrying on in bug 187528 and dependants.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.