Last Comment Bug 433117 - Don't try to import IE favorites on mac
: Don't try to import IE favorites on mac
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: seamonkey2.0a1
Assigned To: Stefan [:stefanh]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-09 17:26 PDT by Stefan [:stefanh]
Modified: 2008-05-24 04:40 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Remove the mac parts (14.84 KB, patch)
2008-05-09 17:26 PDT, Stefan [:stefanh]
mnyromyr: review-
neil: superreview+
Details | Diff | Review
New version (15.01 KB, patch)
2008-05-16 13:09 PDT, Stefan [:stefanh]
mnyromyr: review+
stefanh: superreview+
Details | Diff | Review

Description Stefan [:stefanh] 2008-05-09 17:26:07 PDT
Created attachment 320309 [details] [diff] [review]
Remove the mac parts

If IE on mac wasn't dead and buried someone could have fixed bug 205483 (opened 2003-05-13 04:00:05 PDT).

I don't really know this code, but removing all the mac parts seems to work fine. I've been trying to use "ifdef" instead of "if defined", but I can revert that if someone disagrees.
Comment 1 neil@parkwaycc.co.uk 2008-05-14 08:21:09 PDT
Comment on attachment 320309 [details] [diff] [review]
Remove the mac parts

We'll probably prefer to use the migrator for static bookmark import anyway.
Comment 2 Karsten Düsterloh 2008-05-16 12:31:00 PDT
Comment on attachment 320309 [details] [diff] [review]
Remove the mac parts

>Index: suite/browser/src/nsBookmarksService.cpp
>===================================================================
> #ifdef DEBUG_varga
>     PRTime now;
>-#if defined(XP_MAC)
>-    Microseconds((UnsignedWide *)&now);
>-#else
>     now = PR_Now();
>-#endif
>     printf("Start reading in bookmarks.html\n");
> #endif

You could just do 
  PRTime now = PR_Now();
or maybe even better
  PRTime now(PR_Now());

>-#elif defined(XP_BEOS)
>+#else
>     rv = NS_GetSpecialDirectory(NS_BEOS_SETTINGS_DIR, getter_AddRefs(systemBookmarksFolder));

This is wrong, because it'll enable the BEOS part for all platforms except XP_WIN.

> #ifdef DEBUG_varga
>     PRTime      now2;
>-#if defined(XP_MAC)
>-    Microseconds((UnsignedWide *)&now2);
>-#else
>     now2 = PR_Now();
>-#endif

See above. 

>Index: suite/browser/src/nsBookmarksService.h
>===================================================================
>-#ifdef DEBUG
>-#if defined(XP_MAC) || defined(XP_MACOSX)
>-#include <Timer.h>
>-#endif
>-#endif

I wonder why <Timer.h> was included for XP_MACOSX in the first place, but I guess it's not important anymore anyway.
Comment 3 Stefan [:stefanh] 2008-05-16 13:09:32 PDT
Created attachment 321320 [details] [diff] [review]
New version

(picked the PRTime now = PR_Now(); style in varga's debug blocks)

> >-#elif defined(XP_BEOS)
> >+#else
> >     rv = NS_GetSpecialDirectory(NS_BEOS_SETTINGS_DIR, getter_AddRefs(systemBookmarksFolder));
> 
> This is wrong, because it'll enable the BEOS part for all platforms except
> XP_WIN.
> 
 The code is in a "#if defined(XP_WIN) || defined(XP_BEOS)" block, so it should be enough with this ;-)
Comment 4 Stefan [:stefanh] 2008-05-16 17:36:00 PDT
(In reply to comment #2)

> >Index: suite/browser/src/nsBookmarksService.h
> >===================================================================
> >-#ifdef DEBUG
> >-#if defined(XP_MAC) || defined(XP_MACOSX)
> >-#include <Timer.h>
> >-#endif
> >-#endif
> 
> I wonder why <Timer.h> was included for XP_MACOSX in the first place, but I
> guess it's not important anymore anyway.

It must have been for this (etc):

>-#if defined(XP_MAC)
>-    Microseconds((UnsignedWide *)&now);
>-#else

See http://developer.apple.com/documentation/Carbon/Reference/Time_Manager/Reference/reference.html

Comment 5 Karsten Düsterloh 2008-05-22 15:44:26 PDT
Comment on attachment 321320 [details] [diff] [review]
New version

Seems to work fine here, although ...

>Index: suite/browser/src/nsBookmarksService.cpp
>===================================================================
>+    PRTime now2; = PR_Now();

... Jan probably won't like this build error. ;-)

r=me with that fixed on checkin.
Sorry for misreading your last patch. :-(
Comment 6 Stefan [:stefanh] 2008-05-23 11:34:20 PDT
Landed without incorrect ";":

Checking in suite/browser/src/nsBookmarksService.cpp;
/cvsroot/mozilla/suite/browser/src/nsBookmarksService.cpp,v  <--  nsBookmarksService.cpp
new revision: 1.353; previous revision: 1.352
done
Checking in suite/browser/src/nsBookmarksService.h;
/cvsroot/mozilla/suite/browser/src/nsBookmarksService.h,v  <--  nsBookmarksService.h
new revision: 1.43; previous revision: 1.42
done

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