147620 - Wisner patch.

Status: VERIFIED INVALID

(Camino Graveyard :: General, defect)

Description

[William Dell Wisner]

This patch:

* makes the Home button load the home page (and reorganizes a couple of methods
to facilitate this)
* fixes a handful of irrelevant compiler warnings
* correctly fixes SystemConfiguration error checking
* adds some preference checks that hook into a new preference pane I'll be
posting to Bugzilla separately
* fixes a minor bug or two

Comment 1

[William Dell Wisner]

Attachment: Wisner diff

Comment 2

[saari (gone)]

William, in the future please try to keep unrelated fixes in different
bugs/patches. It just makes reviewing, tracking and verification easier. Thanks!

Comment 3

[Winnie Lam]

Do we want the Home button for Preview?

Comment 4

[William Dell Wisner]

The Home button is already there (though not visible by default); it just
doesn't do anything.

Oh, and sorry about that.

Comment 5

[Mike Pinkerton (not reading bugmail)]

try to keep a bug/patch to one issue at a time. however...

2 spaces per tab, everywhere. right now you're using 4. also, i'd prefer not to
see braces around a single expr, such as
  if ( foo ) {
    x = y;
  }
just do 
  if ( foo )
    x = y;

-  PRBool reuseWindow = PR_FALSE;
+  PRBool boolPref = PR_FALSE;

why did you make this change? it only serves to make the code harder to read to
save one additional bool on the stack. 

-  if ([[mPreferenceManager homePage] isEqualToString: @"about:blank"])
-    [controller focusURLBar];
-  else
-    [[[controller getBrowserWrapper] getBrowserView] setActive: YES];

are you sure you want to remove this code? i don't see where the focus is set
appropriately in your version

+-(BrowserWindowController*)openBrowserWindow
+{
+    BrowserWindowController* browser = [[BrowserWindowController alloc]
initWithWindowNibName: @"BrowserWindow"];
+    [browser home];
+    [browser showWindow: self];
+    return browser;
 }

again, where is the focus? it should be in the url bar if the home url is
about:blank, otherwise on the webpage.

ndex: ProgressDlgController.mm
===================================================================
RCS file: /cvsroot/mozilla/chimera/ProgressDlgController.mm,v
retrieving revision 1.6
diff -u -r1.6 ProgressDlgController.mm
--- ProgressDlgController.mm	23 May 2002 11:19:51 -0000	1.6
+++ ProgressDlgController.mm	28 May 2002 15:35:55 -0000
@@ -49,7 +49,7 @@
 #include "nsIComponentManager.h"
 #include "nsIPrefBranch.h"
 
-const char* prefContractID = "@mozilla.org/preferences-service;1";
+static const char* prefContractID = "@mozilla.org/preferences-service;1";
 
 class nsDownloadListener : public nsIWebProgressListener
 {

why? you never use this contract id in this file.

-- (BOOL)applicationShouldHandleReopen:(NSApplication *)theApp
hasVisibleWindows:(BOOL)flag

why did you remove this?

this patch also has the pb changes for another bug, yet none of the source
files. can you try to migrate things so that the appropriate patches are all
together? file multiple bugs if you have to.

Comment 6

[William Dell Wisner]

About ProgressDlgController.mm: prefContractID was there when I started. I made 
it static because it was bumping into the symbol from other modules. If it's 
not used in ProgressDlgController.mm, nuke it.

I've added the PB changes to the relevant bug.

I will bundle up the remaining changes separately and file them as individual 
bugs as appropriate. Please feel free to close out this one.

Comment 7

[Mike Pinkerton (not reading bugmail)]

closing out per request. the |prefContractID| comment was a mistake. it does
appear to be used. no matter.

Comment 8

[sairuh (rarely reading bugmail)]

zap.