All users were logged out of Bugzilla on October 13th, 2018

Two copies of nsIPasswordManager.idl in the tree confuses CodeWarrior 6

VERIFIED FIXED in mozilla0.9.1

Status

P3
normal
VERIFIED FIXED
18 years ago
14 years ago

People

(Reporter: lordpixel, Assigned: samir_bugzilla)

Tracking

Trunk
mozilla0.9.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: a=dbaron on 6/1; building branch)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
Currently there are two copies of nsIPasswordManager.idl in the tree:
http://lxr.mozilla.org/mozilla/find?string=nsIPasswordManager

/extensions/wallet/public/nsIPasswordManager.idl 
/netwerk/base/public/nsIPasswordManager.idl 

This confuses CodeWarrior 6, on Mac OS X, it seems to see the wrong one (the 
netwerk one) when building wallet.mcp.

AFAICT from comments in bug 70382 the netwerk one is dead and should be deleted 
from the tree, but I could be wrong. cc:ing people who have checked in these 
files
(Reporter)

Comment 1

18 years ago
BTW, the workaround to build in CodeWarrior 6 is to go into dist/netwerk and 
remove the alias to nsIPasswordManager.h. wallet.mcp builds after netwerk, I did 
this inbetween building the two. I don't know if netwerk.mcp will build without 
this alias being there. If this is indeed a dead file, then I suspect it will.
Blocks: 53682

Comment 2

18 years ago
Sounds like your work-around might be the correct fix.  So is this a change to 
the .mcp file and, if so, can you check this change into the tree?
That is not the right fix.  The right fix is to move the PasswordManager IDL
from wallet to netwerk, so that we can get rid of the insanely stupid dependency
on an ``extension''.

Can you produce a patch that goes the other way?

Depends on: 18532
(Reporter)

Comment 4

18 years ago
Produce a patch? Hmmm, I fear not, I can't checkin.

I'm guessing that you'd want to:

i/   remove the old idl file from netwerk
ii/  move the one from wallet to netwerk
iii/ edit walletIDL.mcp to remove nsIPasswordManager.idl (which is now gone)

I can make the change to walletIDL.mcp, but it won't do you any good unless you 
have a mac to check it in from.

Updated

18 years ago
Priority: -- → P3
Target Milestone: --- → mozilla1.0

Comment 5

18 years ago
I'm not sure why this is really assigned to jj.
This bug is really in wallet/netwerk.
Someone else should take this bug and fix it since jj is on vacation for several 
weeks.

Comment 6

18 years ago
Adding self to cc-list.

The question is why does nsIPasswordManager.idl need to be build in netwerk. If
necko needs nsIPasswordManager.h in order to build, then nsIPasswordManager.idl
needs to live in the mozilla/netwerk/ hierarchy, end of story. Having it live in
mozilla/extensions/wallet/ when it's required by necko is asking for trouble,
let alone leaving a nasty taste in my mouth.

If Necko isn't dependent on nsIPasswordManager.h, then that's easy. (Everyone
nod their head)

Comment 7

18 years ago
Could we stick to one issue here, namely the fact that the wrong version of 
nsIPasswordManager.h is being accessed on the mac.  As far as I can tell, that 
probably involves some trivial change in a mac project file.

The other issue, as to whether to reorganize password manager (also cookie 
manager for that matter) so that it no longer appears in an extensions directory 
is a much more global issue that should be addressed in a separate bug report.
(Reporter)

Comment 8

18 years ago
* It got assigned to jj because it was originally under build-config and I
noticed cls bounces all Mac build config bugs to jj, so I've started to save him
the trouble.

* The issue here is there are 2 idl files in the tree which are both called
nsIPasswordManager.idl. They have different contents. Two interfaces with the
same name but different contents seems like a Bad Thing to me, but I don't have
the experience to know for sure.

* I don't think changing a Mac project file can resolve this issue. Shaver
pointed out to me that we have a general rule against 2 files with the same name
in the tree as it confuses Mac builds. I now understand why:

networkIDL.mcp builds its copy of nsIPasswordManager.idl and places
nsIPasswordManager.h in dist/netwerk/nsIPasswordManager.h

then walletIDL.mcp builds its copy of nsIPasswordManager.idl and places
nsIPasswordManager.h in dist/wallet/nsIPasswordManager.h

Since Mac projects are set to include "dist" in their header search path (Access
Paths) any project which does #include "nsIPasswordManager.h" appears to now be
playing a game of chance as to which version it sees. Aparently CodeWarrior 5
gets one and 6 gets the other.

Therefore I believe we need to eliminate the fact we have 2 incompatible copies
of this idl file in the tree.

Whether this is a Mac build issue only, or whether that is merely a symptom of
the real problem (that we shouldn't have the 2 different copies of the "same"
idl file?) I leave up to you guys...

Comment 9

18 years ago
Absolutely, there should be only one version.

Prior to my recent changes to idl-ize the password manager, there was only one 
version and that was in netwerk.  Forthermore, it was never used.

I needed a name similar to this so I chose nsIPswdManager.idl in order to avoid 
problems such as the one we are now facing.  Valeski objected saying that the 
longer name (nsIPasswordManger.idl) made more sense and since it was never being 
used we should claim the name.  So that is what was done.

The way things stand now, there is a version in netwerk that is never being 
used, at least not on linux and win32.  It may be that there are some problems 
in mac project files which are still pulling in the deprecated version from 
netwerk and, as such, those project files need to be fixed.

Comment 10

18 years ago
Why can't we remove the version in netwerk?  Is someone blocking that from
happening?  I'll fix this if someone will give the OK (who is module owner for
this file?).
(Reporter)

Comment 11

18 years ago
If you remove this file from the tree, reference to it needs to be taken out of
the NetwerkIDL.mcp CodeWarrior project file too.

Comment 12

18 years ago
cathy, you have my OK as module owner of the real password manager to remove the 
defunct one from netwerk.  And, yes, you need to update the project file at the 
same time of course.
Wait a sec.  The whole point behind checking in the copy in netwerk was to
eliminate some of the stupid dependencies that our product has on wallet.  (See
18352 for the history: moving wallet and cookie manager to a non-extensions
directory is a non-solution, Steve, and you know it.)

Please copy the updated version into netwerk/ and remove the one in
extensions/wallet/.
Depends on: 18352
No longer depends on: 18532

Updated

18 years ago
Blocks: 83331

Comment 14

18 years ago
*** Bug 83331 has been marked as a duplicate of this bug. ***

Comment 15

18 years ago
carryover milestone from duplicate bug
Target Milestone: mozilla1.0 → mozilla0.9.1

Updated

18 years ago
Assignee: jj → morse
Component: Build Config → Password Manager
OS: MacOS X → All
Hardware: Macintosh → All

Comment 16

18 years ago
Steve Morse needs to own this bug, and resolve the conflict between the two 
nsIPasswordManager.idl files in the tree.
how long will this take to fix? what is the user impact? Why has this only come 
onto the radar right now - it sounds like this problem existed for a while - why 
is it critical to fix before the Netscape beta? thanks. 
Keywords: nsbeta1
Whiteboard: no eta

Comment 18

18 years ago
From 83331:

xptLink returns the following error when executed as part of the mac packaging 
automation:

> Linking .xpt files...
> [browser]
> ERROR: found duplicate definitions of interface nsIPasswordManager with iids
> 173562f0-2173-11d5-a54c-0010a401eb10 and 110c808c-1dd2-11b2-8de5-814d4485c444
> # Error: xpt_link failed.  Exiting...

This causes all .xpt files under Components to be packaged as individual files 
instead of being merged into browser.xpt, mail.xpt, etc.

===============

Linking all the .xpt files together into one file is a performance improvement.
 I don't know if a regression and mac performance improvements qualify this for
beta, but I would think it should be fixed before RTM.
--> samir
Assignee: morse → sgehani
*** Bug 83331 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 21

18 years ago
Should this be nsbeta1+?  Not sure what process dictates these days.
Status: NEW → ASSIGNED
(Assignee)

Updated

18 years ago
Whiteboard: no eta → Plan to have fix ready by the evening of 05/31/2001
(Assignee)

Comment 22

18 years ago
Gagan, 
Is it OK to stop building mozilla/netwerk/base/public/nsIPasswordManager.idl on
all platforms?  And to cvs remove this file?  
(Assignee)

Comment 23

18 years ago
Gagan, 
Please ignore my last question.

JJ has clarified that this is not as trivial as I had assumed it was based on a
verbal discussion when taking ownership of this bug.  Should have read the bug.
 :o)  I will try to turn lordpixel's and shaver's comments into a working patch.  

Comment 24

18 years ago
NNNNOOOOOO!!!!!

Please read shaver's comments.
(Assignee)

Comment 25

18 years ago
Dougt, 
Please read mine.  :o)
Making this nsbeta1+. 
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Updated

18 years ago
Whiteboard: Plan to have fix ready by the evening of 05/31/2001 → Fix should be ready by morning of 06/01
(Assignee)

Comment 27

18 years ago
Created attachment 36825 [details] [diff] [review]
Patch to move nsIPasswordManager.idl from extensions/wallet/public to netwerk/base/public
(Assignee)

Comment 28

18 years ago
(*) I moved the extensions/wallet/public/nsiPasswordManager.idl contents to 
netwerk/base/public/nsIPasswordManager.idl.

(*) Not in the patch: I removed nsIPasswordManager.idl from walletIDL.mcp and 
will cvs rm extensions/wallet/public/nsIPasswordManager.idl.

morse, please r.
shaver, please sr.
(Assignee)

Updated

18 years ago
Whiteboard: Fix should be ready by morning of 06/01 → Fix in hand; need r, sr, a

Comment 29

18 years ago
If you've tested it and it works, r=morse
(Assignee)

Comment 30

18 years ago
Indeed I tested it.  I neglected to mention this in my comment above but here's
what I did:
1> Go to home.netscape.com's webmail link in the "netscape hat" atop the page.
2> Login with my dummy netscape.net account.
3> Ask password manager to save the password.
4> Log out and relogin.
5> Password manager prefilled my password in the login page.

Steve,
Is this adequate?  Any other tests I should run to be thorough?  Thanks for the
review.
(Assignee)

Updated

18 years ago
Whiteboard: Fix in hand; need r, sr, a → Fix in hand; have r; NEED sr, a

Comment 31

18 years ago
The test you want is to bring up the password-manager dialog 
(tasks->privacy->password-manager->view-saved-passwords) and see if your saved 
passwords are displayed.
(Assignee)

Comment 32

18 years ago
Yes, passed the test in the comment above this mentioned by morse.

Comment 33

18 years ago
sr=sfraser. You're going to cvs remove the obsolete file, and do the right IDL 
project foo, I assume.

Comment 34

18 years ago
ooh goody, since i just ran into this silly stuff again while foolishly 
building --with-extensions=transformiix
Whiteboard: Fix in hand; have r; NEED sr, a → Fix in hand; have r, sr; NEED a
a=dbaron for 0.9.1 and trunk

Updated

18 years ago
Whiteboard: Fix in hand; have r, sr; NEED a → Fix in hand; have r, sr; a=dbaron
(Assignee)

Updated

18 years ago
Whiteboard: Fix in hand; have r, sr; a=dbaron → Fix in hand; have r, sr, a=dbaron; building branch

Comment 36

18 years ago
Is this checked in yet?  Anything holding it up?
Whiteboard: Fix in hand; have r, sr, a=dbaron; building branch → a=dbaron on 6/1; building branch
(Assignee)

Comment 37

18 years ago
My branch build was holding this up and the fact that I still have 4 other
nsbeta1+ bugs (3 are MUST HAVE).  I am batch processing to be efficient.  :o) 
Checking in now.

Comment 38

18 years ago
Thanks for your patience (I've just been noticing how many of these depend on
you :-)
(Assignee)

Comment 39

18 years ago
Checked in on branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 40

18 years ago
QA to jj to verify since he was seeing this on the the verification mac.
QA Contact: granrose → jj

Comment 41

18 years ago
Verified that there is only 1 copy of nsIPasswordManager.idl (under mozilla/
netwerk/base/public/).
Xptlink doesn't fail since 06/05 (see bug 83331 marked verified that date)
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.