212630 - [PATCH] Bookmark system rewrite

Status: VERIFIED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[David Haas (gone, not reading mail)]

Rewrite of the bookmark system.  Also includes various cleanup of include files,
headers, etc.  A memory leak or two gets fixed also.

Comment 1

[David Haas (gone, not reading mail)]

Attachment: edited file patch

patch contains a list of files which a) remain in browser b) were edited.

Comment 2

[David Haas (gone, not reading mail)]

Attachment: new files

this is a tar'd, gzip'd file containing the new source files added to the
project.

Comment 3

[David Haas (gone, not reading mail)]

Attachment: modified nibs

This tar'd, gzip'd file contains 1 new nib (BookmarkImportDlg.nib) and a
modified BookmarkInfoController.nib & BrowserWindow.nib

BookmarkImportDlg & BookmarkInfoController I think are fine.  BrowserWindow.nib
has more than a little goofiness with the add folder, add bookmark, and add
separator buttons in BookmarkView.  Somebody may want to figure out what's
going on there - I tried for an hour then gave up.

Comment 4

[David Haas (gone, not reading mail)]

CC'ing pinkerton

I hope you find this useful.

Instructions for building:  

1st: delete everything in your current camino/src/bookmarks folder.  It's all
been replaced.  

2: apply the "edited file patch"

3: take all the files in "new files" and import them into the Camino project. 
Add everything to the "Camino" and "CaminoStatic" targets ***EXCEPT*** the files
"AddressBookManager.h" and "AddressBookManager.m" 

4: Make a new target in the Camino project - call it "AddressBookManager", make
it a bundle.  I used "org.mozilla.camino.AddressBookManager" as the identifier
and "AddressBookManager" as the principal class.  Add the files
"AddressBookManager.h" and "AddressBookManager.m" to this target.  Also add the
"Address Book" framework to THIS TARGET ONLY.  Otherwise, it won't launch on 10.1.

5: Unpack the nibs in the "modified nibs" .tgz file.  The BookmarkInfoPanel nib
is a complete replacement of the current nib file by the same name.  The
BookmarkInportDlg nib is a new nib file to handle imports in a slightly more
friendly manner.  These should get dumped right in the project.

BrowserWindow.nib has, as I mentioned in a previous comment, some goofiness with
the "add bookmark" "add folder" and "add separator" buttons.  They seem to move
around.  I don't know why.  Somebody may want to figure out what's happening
there.  Otherwise, it's an updated for the new bookmark system (and cleaned up)
version of the current BrowserWindow.nib file.  

I tried, whenever possible, to throw away any remnants of the sidebar bookmark
system.

I've gotten to the point where it works - it's not perfect, by any strech of the
imagination, but it works.  It needs icons for the "bookmark menu separator"
button, and a "broken bookmark" and "updated bookmark" icon would be very nice
as well.  Plus special icons for the rendezvous, address book, top 10 list, etc
folders.  Update checking doesn't bother looking for a proxy.  The update
checking happens on a background thread for no reason other than I wanted to
make it threaded.  It'd be nice if it did smart replacement of context menu
items.  I didn't bother including the scriptSuite or scripTerminology files
(still playing with those).  Etc, etc, etc.  

I make no guarantee that I've included all the files you need to build.  I make
no guarantee that the aren't still bookmark corruption bugs in this code (since
I found some just a few hours ago).  

And let me apologize right now for the multiple e-mails coming from this bug all
in a big flood.

Comment 5

[David Haas (gone, not reading mail)]

Attachment: icons

forgot - Jasper Hauser sent me these icons.  They might be useful.

Comment 6

[Mike Pinkerton (not reading bugmail)]

from the edited file patch.....

-  IBOutlet BookmarksToolbar *mBookmarksToolbar;
+  IBOutlet BookmarkToolbar *mBookmarksToolbar;

seems like a gratuitous rename that will cause a much bigger diff than
necessary. is this really needed? The same is true of BookmarkManager.

-- (BookmarksToolbar*) bookmarksToolbar;
+- (BookmarkToolbar*) BookmarkToolbar;

why change this to a leading cap?

+//#import "DHBookmarksDataSource.h"
+//#import "DHBookmarksController.h"

can these be removed?

+      // There might be a race here between
+      // the bookmark manager loading on its startup
+      // thread and the first window opening.  
+      [mPersonalToolbar buildButtonList];

what can we do to prevent this race? is a lock necessary? the minute we think
it's not much to worry about, it will be.

-  [mBookmarksController selectContainer:kBookmarksMenuContainer];
+  [mBookmarkViewController selectContainer:kBookmarkMenuIndex];

how about |kBookmarkMenuContainerIndex| ? :)

-    NS_ConvertUCS2toUTF8 utf8URL(url);
+//    NS_ConvertUCS2toUTF8 utf8URL(url);
     nsCOMPtr<nsIURI> fixedURI;
-    mURIFixer->CreateFixupURI(utf8URL, 0, getter_AddRefs(fixedURI));
+    mURIFixer->CreateFixupURI(url, 0, getter_AddRefs(fixedURI));

we don't need to convert to utf8?

-  [mPreferenceManager release];
+  [PreferenceManager sharedInstance];

i don't understand what's happening here.

+  // dock bookmarks. Note that we disable them on 10.1 because of a bug noted here:
+  //
http://developer.apple.com/samplecode/Sample_Code/Cocoa/DeskPictAppDockMenu.htm

i know you just moved code here, but is this for real?

+     rootDHBookmarkFolder: [BookmarkManager bookmarkMenuFolder]];

What does the DH stand for here?

+    mDockBookmarks = [[BookmarkMenu alloc] initWithMenu: mDockMenu
+                                                 firstItem: firstBookmarkItem
+                                      rootDHBookmarkFolder: [BookmarkManager
top10Folder]];

what about all the people who have already set a dock bookmarks folder? do we
always init it to the top 10 folder?

+// this gets called by bookmark load background thread, so it's a possible
+// point of contention.  but, it's only called once on startup, so probably
+// won't be a problem.

so this is called not on the main thread, does anything called from this end up
making calls into parts of appkit that aren't threadsafe? just checking.

+  if ([dirtyStringMutant
respondsToSelector:@selector(replaceOccurrencesOfString:withString:options:range:)])
+      {

don't extra indent. the open brace should be on the same line as the |if|.

looking very promising, will review more of the new code later.

Comment 7

[David Haas (gone, not reading mail)]

Hey - if you're online now, IM me @ davebo95

Otherwise, I'll start fixing things.  Do you want a patch to the patch, or just a new patch?

Comment 8

[Mike Pinkerton (not reading bugmail)]

fwiw, i don't think any file in the FE needs a prefix (we're not going to
namespace collide with ourselves). I think the bookmarks/DH* files should just
drop the prefix.

+ @interface AddressBookManager : NSObject {
+  id mAddressBookFolder;
+}

two nits, interfaces don't have the open brace on the same line. Yeah, we're
inconsistent, but that's the way it is. :) Also, can this be something other
than an |id|? Stronger type checking is always good.

//Update Frequency Flags
... 

//Status Flags
... 

these should be enums if this is a c++ header file.

+ @interface Bookmark : BookmarkItem  //DBME

what do these acronyms mean? DBME? DBMu? 

  if (!aURL || [mURL isEqualToString:aURL]) //think this is OK - but non-reentrant?
    return;
  NSString *curURL;
  [aURL retain];
  [mLock lock];
  curURL = mURL;
  mURL = aURL;
  [mLock unlock];

probably move the isEqualToString into the lock? is it important? also, rewrite
that assignment of the temp into one line (it's c++):

 NSString* curURL = mURL;

+  if (!myURL) {
+    NSLog(@"%@ can't form NSURL, turning off update checking",[self url]);
+    [self setUpdateTimeUnits:kBookmarkNeverUpdate];
+  }

does this get reset if the user edits the url manually?

+ NSString *DHBMTitleKey = @"Title";
...

again, no need for the prefix

ok, my battery is dying....more later.

Comment 9

[David Haas (gone, not reading mail)]

Attachment: new files

tar'd, gzip'd version of revised new bookmark files

Comment 10

[David Haas (gone, not reading mail)]

Attachment: modified nibs

modified nibs.	as I mentioned before, browserwindow.nib has some funky stuff
going on in BookmarkView so shouldn't be added until that's cleaned up.

Comment 11

[David Haas (gone, not reading mail)]

Attachment: edited file patch

patch with edited files

Comment 12

[David Haas (gone, not reading mail)]

Attachment: new files I forgot to add earlier

more new files - the extensions NSArray+Utils & ExtendedTableView

Comment 13

[David Haas (gone, not reading mail)]

Attachment: patch to localized strings

forgot to add this one, too.

Comment 14

[David Haas (gone, not reading mail)]

Also - just to get things workinging nicely,  you'll need to add the following
tif files to the available resouces.  

brokenbookmark_icon.tif
rendezvous_icon.tif
updatedbookmark_icon.tif
top10_icon.tif
bookmarktoolbar_icon.tif
bookmarkmenu_icon.tif
addressbook_icon.tif

I used (slightly renamed) files from the "icons" set of files above.

Comment 15

[David Haas (gone, not reading mail)]

finally, just to add yet more spam to your mailboxes:

build instructions as as mentioned in comment 4.

*I added back in Dock Menu support as an attribute.
*I fixed (I think) the stuff mentioned in comments 6 & 8.
*You can search bookmarks :)
*The search field is useless for organization - it's just good
for finding (in other words, it'll tell you if you have 10 bookmarks
pointing to Google, but it won't tell you where they all are)

That's about it.

Comment 16

[S Woodside]

Have you synchronized this with the latest cvs?

Comment 17

[Jasper]

Attachment: Updated icons.

Updated icons. Including an entirely new brokenbookmark icon.

Comment 18

[S Woodside]

re comment #13 does this strings file look right to you? I see a lot of wonky
characters when I download it.

Comment 19

[S Woodside]

I installed this (everything but the localization strings) and it builds and
runs without any immediately obvious problems. I'll live on it for a while and
let you know if I have any troubles ok?

Comment 20

[Mike Pinkerton (not reading bugmail)]

thanks simon, i'm still slogging through the new files. this is a bad week for
me, hopefully next week will be better.

dave: sorry to take so long on this, i'm just really busy :(

Comment 21

[David Haas (gone, not reading mail)]

wow - just had a mid air collision.  didn't know that could happen.

the funny characters in localized strings probably come from the fact that it's a unicode file.  your 
best bet is to just copy & paste the new strings from the patch into localizable.strings in PB.

Glad to know it built.

Comment 22

[S Woodside]

No crashes so far. It's an improvement. I don't like the updates feature, I
tried it with slashdot and it beeps every time it checks because they always
change little bits of it.

Comment 23

[Mike Pinkerton (not reading bugmail)]

the update feature is something i've been mulling over. I really don't want to
add full functionality here, it's better left to other utilities like SurfTron
which have lots of complicated code for comparing content sizes and parsing of
the actual html to determine if changes are only in urls, etc.

If we can get like 90% with minimal checking of the http headers, then I say
it's worth it, but if we can't (and I don't think we can), then we shouldn't
take the hit. If it doesn't appear to really work right, it'll just be a black
eye, IMHO.

I'm open to dissuasion.

Comment 24

[Jasper]

I made a list of all the bugs I encoountered and enhancement request I could
think of using a on official build, containing the files uploaded here,made by
S. Wooodside.

http://www.jasperhauser.nl/camino/new_bm_bugs.html

Comment 25

[David Haas (gone, not reading mail)]

I added bookmark checking just because Omniweb has it, and I had read "oooh, Omniweb has the 
best bookmark system, 'cause it'll check to see if your bookmarks are updated."

Omniweb's checking, as near as I can tell, works EXACTLY the same - just by checking the reply 
code on a HEAD request for "200".  Unfortunately, lots of sites these days will return 200 on a 
check even if the content hasn't really changed.  So it is kind of useless.  I thought about giving it 
an extra field where you could enter a RSS link, and it would check that instead, but that's 
NetNewsWire.  

If you pull it, it makes the code somewhat simpler.  And you lose the need for that 
RunLoopMessenger class.  

Comment 26

[Simon Fraser [no longer active]]

Would it be over the top to allow the user to specify update checking
per-bookmark? It could be a checkbox in the Add Bookmark dialog, and in the Get
Info window, labelled "Notify me when this page changes".

Comment 27

[David Haas (gone, not reading mail)]

that's how it works now, so no, it wouldn't be over the top.

Comment 28

[Jasper]

I tested this sytem and most big sites (macnn, versiontracker, etc.) claim to be
updated while they are not. So as David said in Comment #25 it is quite useless.
Accept for home maid (non php sql) pages. The ideal situation would be to have a
more intelligent system. 

Most people want to able to use in with every site succesfully.Either we do it
right or we don't. We don't want to make poeple think it works perfectly as they
expect it would and then let them discover it only works in some situations.

But I must say that the reachable/unreachable feature is very usefull for the
obvious reasons.

Oh ehm david, if you have time please read the the list of bugs I attached in
Comment #24. It might help.

Comment 29

[Mike Pinkerton (not reading bugmail)]

yeah, i think that doing "updated" right may be an unobtainable goal, but it
would be nice to know sites in my bm list that are no longer valid
(dead/moved/etc). that's mainly what i was thinking when I envisioned how we'd
use such a feature.

Comment 30

[David Haas (gone, not reading mail)]

Attachment: new bookmark files

OK - new bookmark files, updated to get rid of update checking.

Comment 31

[David Haas (gone, not reading mail)]

Attachment: modified nibs

yet again, modified nibs.  still funkiness in BrowserWindow.nib with the
placement of the buttons.

Comment 32

[David Haas (gone, not reading mail)]

Attachment: edited files patch + localized stirngs + new default bookmark file

Just a little change in CHBrowserListener from before, plus some new strings in
localized strings & the new default bookmark file, which is just like the old
default bookmark file but now as a plist.

Comment 33

[David Haas (gone, not reading mail)]

changes in BookmarkManager.mm depend on patch for bug 181752 landing.  Sorry.  

Comment 34

[Jasper]

David you fixed bug 187420 in your lates nib update.

Comment 35

[Jasper]

Attachment: Modified bookmarks info panel.

Modified the bookmarks info panel to match the layout style used in the Camino
prefernce panels.

Comment 36

[Jasper]

Attachment: Updated icons (rendezvous icon).

Updated the rendezvous icon so it looks OK again when it's seletcted.

Comment 37

[David Haas (gone, not reading mail)]

Attachment: new bookmark files

OK - I think this qualifies as a releasable bookmarks 2.0
* searching works.  and double clicking on a found bookmark does something.
* after searching, pick a bookmark & press magnifying glass button highlights
the bookmark in the outline view pane.
* bookmark status checking uses http proxy, if necessary
* bookmark status checking does 1 bookmark ever 2 minutes.
* bookmark status checking uses something resembling a corret user-agent string

* clearing # visits on a bookmark in top 10 list works sensibly
* icons in bookmark menu work again.

Comment 38

[David Haas (gone, not reading mail)]

Attachment: browser window nib

This is a more-correct BrowserWindow.nib file than you can find in the last
"modified nibs" .tgz file.  But, the separator button positioning is still
totally screwed up, so somebody's going to have to fix that.

Comment 39

[Jasper]

Attachment: Modified bookmarks manager icons

Some minor changes.

Comment 40

[Jasper]

Attachment: Modified bookmarks manager icons.

Added a separator icon (it's styled in the current theme).

Comment 41

[Jasper]

Attachment: Modified bookmark manager icons.

This package now also contains a delete_on icon, the one in the that is
available now is broken.

I'm also requesting to change the title of the edit button into the info button
since the the window opened by the edit button is actually the info window. And
since most editing in the new manager will be done in the bookmarks view and
the info panel only shows the user extra info and settings for the bookmark, I
belive it's best and more clear to use another name and icon for this button.

So this package also contains a info_on icon.

Comment 42

[S Woodside]

I've got this latest stuff going now. Some quibbles:
- if I quit and restart the find panel is minimized. this is good. What is not
good, is that if I enter a search string and hit return, the panel does not pop
up by itself
- what is this button labelled "S" supposed to do? also it needs an icon
- the function to locate a result a search result item should be called "Reveal
in Bookmarks"
- I like all the icons, except that first one
- i agree with jasper about changing edit to info
- the search field should be rounded, and behave like the search in Mail.app,
i.e., with a little X button that closes up the search and hides the results
- I like the 10 ten list, but what order is it in?
- what do you call those things on the left, are they sections, or folders, or what?
- I'm not entirely clear on why I would want to have a new folder on the left,
that isn't in the bookmarks menu

I posted a build that has this, the current fix for bug 181752 and the history
speed-up patch in it, simonwoodside.com/cam-sbwoodside-2003-08-08.tgz (temporary)

Comment 43

[Jasper]

Just to let everybody know again, I have created a page that keeps track of bugs
I encountered and enhancements I would like to see. It's updated everytime I get
my hands on a recent build from David. So most bugs Simon posted are mentioned
there.

http://www.jasperhauser.nl/camino/new_bm_bugs.html

Things I think really have to be discussed here are the folowing things:
- the entire search function, how it should function and look
- the implementation also searching in history (dave said it would be hard to do)

Comment 44

[Jasper]

David, while you are at it could you have a look at Bug 186113 and Bug 179600
they both seem to still exist in the build you gave me.

Comment 45

[Jasper]

David could you also have a look at Bug 199197 and Bug 154178.
And it seems that you fixed Bug 155484 with your new bookmarks system patch.

Comment 46

[Mike Pinkerton (not reading bugmail)]

as commented in bug 210520, the site icons in bookmarks hangs camino at startup.
are we sure that's something we want to be doing with this patch, or should we
take that code, stick it into 210520 and debate it there separately?

also, should this patch apply cleanly or is it dependent on the profile folder
renaming patch as well?

Comment 47

[David Haas (gone, not reading mail)]

I have a sort-of fix for that.

Rather than firing off the icon load for ALL the icons after camino starts in
one burst, I fiddled around so that the loads get staggered over 10-15 seconds.
 I am just too lazy to make another gigantic patch/zip file for those 10 lines,
though . . 

Comment 48

[S Woodside]

Mike: is the patch ready to go otherwise? That would affect whether to split or
not. Dave: That seems like a good approach generally speaking. What about
concurrancy bugs? What happens if I try to look at the bookmarks before the icon
is loaded. Then look at it again after, etc. etc.

Comment 49

[David Haas (gone, not reading mail)]

Here's what I've seen with the staggered icon loading.  Assuming it's a site
with a site icon, here's what happens:

If you look at a menu before its icon load has fired, you see the default icon.
If you look at a menu after its icon load has fired, you see the site icon.
If you're looking at a menu during the icon load, you get a visual glitch
(that's the best way to describe it).  The glitch is gone the next time you look
at the menu.

And yes, the bookmark stuff now depends on the profile patch, which doesn't seem
to be satisfactory yet.  

When it gets time to land this stuff, ping me and I'll provide a new version of
BookmarkManager.mm which incorporates the staggered icon loading.

Comment 50

[S Woodside]

If we can get bug 181752 integrated, I'd like to see this go in ASAP. What I
would like to see happen: remove the site icon loading routines from this patch.
Create a new bug for loading the site icons and hash out a plan in there, which
will be a subsequent patch to this code once it's in the build.

Comment 51

[S Woodside]

bug 181752 is going to take more time. If the patch for this bug can be made to
be independent of bug 181752, I believe that will allow greater progress to be made.

Comment 52

[Ludovic Hirlimann [:Usul]]

adding haspatch keyword as per new review process.

Comment 53

[S Woodside]

Attachment: Patch tarball, no dependency

This patch is a modified version of David's last patch. It has no dependency on
any other bug. The diff is against today's trunk CVS. The patch is a tarball
with the patch file, all the source files, the nibs, and the images from
Jasper. There's an instruction file inside the tgz to explain how to install
the patch.

Comment 54

[S Woodside]

Comment on attachment 132899 [details]
Patch tarball, no dependency

Setting review? Per the new instructions from pinkerton.

Comment 55

[Josh Aas]

sbwoodside - could you be a little more specific in your instructions? I tried
to apply the patch but the build didn't work. Unfortunately I don't have the
error any more... Here are my questions:

- add resources/images to the directory and to the project (Camino, CaminoStatic)

To the bookmarks directory?

- add resources/localized/English.lproj/BookmarkImportDlg.nib to the directory
and to the project (targets Camino, CaminoStatic)

To what directory?

- add all of the patch's src files into the src directory

Don't add the bookmark files to the bookmark folder? Seems like if you're
dumping all of those files into the src directory, they're going to need to be
put into appropriate subdirectories like "bookmarks" and whatnot.

Comment 56

[S Woodside]

>- add resources/images to the directory and to the project 
> (Camino, CaminoStatic)
> To the bookmarks directory?

The patch organizes the files with a parallel directory structure to what exists
in mozilla/camino. So, add the files to the directory
mozilla/camino/resources/images  (where mozilla/ is your local mozilla source root)

>- add resources/localized/English.lproj/BookmarkImportDlg.nib 
> to the directory and to the project (targets Camino, CaminoStatic)
> To what directory?

copy BookmarkImportDlg.nib to mozilla/camino/resources/locallized/English.lproj/
 and then go to the project in PB, Add Files..., choose BookmarkImportDlg.nib
and add it to the project. Select the targets Camino, CaminoStatic.

> - add all of the patch's src files into the src directory
>
> Don't add the bookmark files to the bookmark folder? Seems 
> like if you're dumping all of those files into the src 
> directory, they're going to need to be put into appropriate 
> subdirectories like "bookmarks" and whatnot.

You can do that if you like. It doesn't matter where they are as long as you can
find them.

Comment 57

[David Haas (gone, not reading mail)]

Attachment: Updated tarball, no dependencies

I updated the bookmark files in Simon's patch to what I currently have.  I hope
nothing breaks.

Comment 58

[S Woodside]

Comment on attachment 132899 [details]
Patch tarball, no dependency

I'm unobsoleting this patch. given how complex it is to install this patch, I'd
rather not do it again, especially if you don't know if it will build or not.

Comment 59

[S Woodside]

Some data points (for patch 132899) about the source changes. In the patch,
there are 723 lines removed, and 851 lines added. In the src/bookmarks
directory, some of the files are renamed: BookmarksButton.{h,mm} becomes
BookmarkButton.{h,mm}, BookmarksMenu.{h,mm} to BookmarkMenu.{h,mm},
BookmarksToolbar.{h,mm} to BookmarkToolbar.{h,mm}. The diffs between the old and
new names are not complete replacements. 

The other files are BookmarksController.{h,mm}, BookmarksDataSource.{h,mm},
BookmarksExport.{h,mm}, BookmarksOutlineView.{h,mm}, BookmarksService.{h,mm}.
These have been removed and replaced and I think refactored code in new files
named Bookmark* (note the 's' is gone).

Comment 60

[Ludovic Hirlimann [:Usul]]

Comment on attachment 132899 [details]
Patch tarball, no dependency

Sorry for the spam. Simon yopu should set the add.review back to + sorryy.

I have issues with this patch:
1) dialup usesr will have their bookmak checke and this means putting the line
up - that's bad. I know david has a solution and needs to implement it.
2)checking each bookmark every two minutes is too much and is not user
configurable. this will lead to performance problem.
3) patch is apain to apply. Took me sometime to install it and compile. Now
that I have a build version launching it does not work I get those messages in
my console :
nsNativeComponentLoader: autoregistering succeeded
nNCL: registering deferred (0)
2003-10-14 20:55:38.568 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:55:38.598 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:55:38.649 Camino[17029] Unknown class `BookmarksController' in
nib file, using `NSObject' instead.
2003-10-14 20:55:38.670 Camino[17029] Unknown class `BookmarksDataSource' in
nib file, using `NSObject' instead.
2003-10-14 20:55:38.672 Camino[17029] Unknown class 'BookmarksToolbar' in nib
file, using `NSView' instead.
2003-10-14 20:55:38.701 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2d08140>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:55:38.703 Camino[17029] Could not connect the action addBookmark:
to target of class NSObject
2003-10-14 20:55:38.703 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:55:38.703 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:55:38.705 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:55:38.705 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:55:38.705 Camino[17029] Could not connect the action
openBookmarkInNewWindow: to target of class NSObject
2003-10-14 20:55:38.706 Camino[17029] Could not connect the action
openBookmarkInNewTab: to target of class NSObject
2003-10-14 20:55:38.706 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:55:38.706 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:55:38.711 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2d08140>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:55:38.713 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:55:38.713 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x2c94ee0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:55:38.714 Camino[17029] Could not connect the action
changeContainer: to target of class NSObject
2003-10-14 20:55:38.714 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x2c94ee0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:55:38.862 Camino[17029] *** -[NSView setDrawBottomBorder:]:
selector not recognized
2003-10-14 20:55:38.879 Camino[17029] Exception raised during posting of
notification.  Ignored.  exception: *** -[NSView setDrawBottomBorder:]:
selector not recognized
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsChromeRegistry.cpp,
line 3190
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsChromeRegistry.cpp,
line 3190
2003-10-14 20:57:08.113 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:57:08.122 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:57:08.145 Camino[17029] Unknown class `BookmarksDataSource' in
nib file, using `NSObject' instead.
2003-10-14 20:57:08.146 Camino[17029] Unknown class `BookmarksController' in
nib file, using `NSObject' instead.
2003-10-14 20:57:08.147 Camino[17029] Unknown class 'BookmarksToolbar' in nib
file, using `NSView' instead.
2003-10-14 20:57:08.178 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2c98b60>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:57:08.179 Camino[17029] Could not connect the action addBookmark:
to target of class NSObject
2003-10-14 20:57:08.179 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:08.180 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:57:08.180 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:57:08.181 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:08.181 Camino[17029] Could not connect the action
openBookmarkInNewWindow: to target of class NSObject
2003-10-14 20:57:08.181 Camino[17029] Could not connect the action
openBookmarkInNewTab: to target of class NSObject
2003-10-14 20:57:08.181 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:57:08.181 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:57:08.185 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2c98b60>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:57:08.186 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:08.187 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x2c98be0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:57:08.187 Camino[17029] Could not connect the action
changeContainer: to target of class NSObject
2003-10-14 20:57:08.188 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x2c98be0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:57:08.199 Camino[17029] *** -[NSView setDrawBottomBorder:]:
selector not recognized
2003-10-14 20:57:08.216 Camino[17029] *** -[NSView setDrawBottomBorder:]:
selector not recognized
2003-10-14 20:57:49.391 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:57:49.399 Camino[17029] Unknown class BookmarksOutlineView in
Interface Builder file.
2003-10-14 20:57:49.435 Camino[17029] Unknown class `BookmarksController' in
nib file, using `NSObject' instead.
2003-10-14 20:57:49.435 Camino[17029] Unknown class 'BookmarksToolbar' in nib
file, using `NSView' instead.
2003-10-14 20:57:49.440 Camino[17029] Unknown class `BookmarksDataSource' in
nib file, using `NSObject' instead.
2003-10-14 20:57:49.449 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2d37880>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:57:49.451 Camino[17029] Could not connect the action addBookmark:
to target of class NSObject
2003-10-14 20:57:49.451 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:49.451 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:57:49.452 Camino[17029] Could not connect the action
deleteBookmarks: to target of class NSObject
2003-10-14 20:57:49.452 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:49.452 Camino[17029] Could not connect the action
openBookmarkInNewWindow: to target of class NSObject
2003-10-14 20:57:49.453 Camino[17029] Could not connect the action
openBookmarkInNewTab: to target of class NSObject
2003-10-14 20:57:49.453 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:57:49.453 Camino[17029] Could not connect the action
showBookmarkInfo: to target of class NSObject
2003-10-14 20:57:49.456 Camino[17029] *** Illegal NSOutlineView data source
(<NSObject: 0x2d37880>).  Must implement outlineView:numberOfChildrenOfItem:,
outlineView:isItemExpandable:, outlineView:child:ofItem: and
outlineView:objectValueForTableColumn:byItem:
2003-10-14 20:57:49.458 Camino[17029] Could not connect the action addFolder:
to target of class NSObject
2003-10-14 20:57:49.458 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x3b120b0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:57:49.459 Camino[17029] Could not connect the action
changeContainer: to target of class NSObject
2003-10-14 20:57:49.459 Camino[17029] *** Illegal NSTableView data source
(<NSObject: 0x3b120b0>).  Must implement numberOfRowsInTableView: and
tableView:objectValueForTableColumn:row:
2003-10-14 20:57:49.468 Camino[17029] *** -[NSView setDrawBottomBorder:]:
selector not recognized
2003-10-14 20:57:49.471 Camino[17029] *** -[NSView setDrawBottomBorder:]:
selector not recognized
2003-10-14 20:57:56.855 Camino[17029] *** -[NSView showBookmarksToolbar:]:
selector not recognized
2003-10-14 20:57:56.857 Camino[17029] *** -[NSView showBookmarksToolbar:]:
selector not recognized
2003-10-14 20:58:34.572 Camino[17029] App will terminate notification
2003-10-14 20:58:34.983 Camino[17029] Shutting down embedding.
2003-10-14 20:58:34.998 Camino[17029] Window will close notification.
2003-10-14 20:58:35.010 Camino[17029] *** -[NSView windowClosed]: selector not
recognized
2003-10-14 20:58:35.010 Camino[17029] Exception raised during posting of
notification.  Ignored.  exception: *** -[NSView windowClosed]: selector not
recognized
2003-10-14 20:58:35.041 Camino[17029] Window will close notification.
2003-10-14 20:58:35.041 Camino[17029] *** -[NSView windowClosed]: selector not
recognized
2003-10-14 20:58:35.042 Camino[17029] Exception raised during posting of
notification.  Ignored.  exception: *** -[NSView windowClosed]: selector not
recognized
2003-10-14 20:58:35.043 Camino[17029] Window will close notification.
2003-10-14 20:58:35.046 Camino[17029] *** -[NSView windowClosed]: selector not
recognized
2003-10-14 20:58:35.046 Camino[17029] Exception raised during posting of
notification.  Ignored.  exception: *** -[NSView windowClosed]: selector not
recognized
4) Mozilla.org's code style is not used in the patch some #defines are in the
code instaed of being in the .h files.

Comment 61

[S Woodside]

> 1) dialup usesr will have their bookmak checke and this means putting the line
> up - that's bad. I know david has a solution and needs to implement it.

I have a patch for this from david look forit soon.

> 2)checking each bookmark every two minutes is too much and is not user
> configurable. this will lead to performance problem.

I don't think it checks every bookmark every two minutes, but rather, every two
minutes, it checks one boomkark, cycling slowly through the whole list over hours.

Comment 62

[S Woodside]

> 4) Mozilla.org's code style is not used in the patch some #defines are in the
> code instaed of being in the .h files.

let's fix that later

Comment 63

[S Woodside]

taking

Comment 64

[S Woodside]

Attachment: modem fix - apply this as well

Checks to see if it's connected to the internet, before automatically verifying
a bookmark. First install Attachment #132899 [details] then apply this patch to the
result.

Comment 65

[S Woodside]

things to fix later: move the class descriptions out of the license block in
AddressBookManager.mm, KindaSmartFolderManager.mm and RunLoopMessenger.mm

Comment 66

[S Woodside]

Comment on attachment 132899 [details]
Patch tarball, no dependency

looked at all the code, modem patch works.

Comment 67

[Ludovic Hirlimann [:Usul]]

comment #66 : network code is clean. 
Thank's to your instructions simon I've been able to apply the patch correctly.
It build sand run fine. I was tired so I did not test every possibility. Will do
it tonight. For the momment I thinks the patch can be applied.

Comment 68

[S Woodside]

Attachment: detailed instructions

very detailed instructions, verified by Ludovic Hirlimann

Comment 69

[Ludovic Hirlimann [:Usul]]

I've been playong with patch for the last 48 hours. I have one issue, with the
broken bookmark feature.
I added a url to the bookmartk and then deleted the file from my server. It
automatically got in broken, but if i cliked on it it did not load and
diseaperead from broken. Imo it should have stayed inthe broken area.
FOr the rest I had no issues with the patch.

Comment 70

[David Smith]

Using one of the custom builds (2003092700) which has this patch (at least it's
got the import/export bookmarks, bookmark search, "top ten list", etc..., so I
assume it has this patch), I'm getting a problem where when I bookmark a group
of tabs, it appears as a submenu that won't open. When I restart the browser it
then behaves as a normal tab group. I didn't think filing a bug on this would be
a good idea, since afaik it doesn't occur in the non-custom build. I can test
that if needed.

Comment 71

[Mike Pinkerton (not reading bugmail)]

landed (whew!). thanks to everyone who helped with this bug, and to dave haas.
please file any regressions as new bugs.

Comment 72

[Chris Petersen]

Verified with the 2003102211 NB.