there is no reason this interface should derive from the other. This means that anyone who uses nsIWindowMediator must bring in RDF.

that's the C++ side of things, and includes a bunch of documentation and string cleanups that I've had in my tree for months. I'm still scouring LXR to see if there are any other changes required. I'm guessing not from my first pass. I think the only time the RDF interface to this class is used is when the tasklist is built up.

discovered that this breaks at least the following dependencies on rdf: - editor - profile - pref migration so this is a Good Thing. Further inquiries for C++ dependencies hasn't turned up any potential runtime bustage. I was worried about lines like: nsCOMPtr<nsIRDFDataSource> mediator = do_GetService(kWindowMediatorCID,&rv); A second pass over JS doesn't reveal any other problems. Continuing my testing.

ok, this looks good. looking for reviewers and super reviewers.. davidm is the original author so he's not around. CC'ing waterson for some RDF sympathy, danm cuz he does windowlist stuff, jag cuz he likes this stuff.

Comment on attachment 49622 [details] [diff] [review] fix this, plus various string and doc updates >Index: appshell/public/nsIWindowMediator.idl >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/appshell/public/nsIWindowMediator.idl,v >retrieving revision 1.11 >diff -u -r1.11 nsIWindowMediator.idl >--- nsIWindowMediator.idl 2001/09/04 23:30:46 1.11 >+++ nsIWindowMediator.idl 2001/09/17 18:55:18 >@@ -35,19 +34,21 @@ > ... >- iterates over the windows of type inType in the order that they were created. if inType is null >- iterates over all windows. This enumerator returns domWindows. >+ iterates over the windows of type inType in the order that >+ they were created. if inType is null iterates over all "If inType", capital I. >+ void updateWindowTimeStamp( in nsIXULWindow inWindow ); >+ >+ /* */ >+ void updateWindowTitle(in nsIXULWindow inWindow, in wstring inTitle ); >+ >+ /* z-ordering: */ >+ >+ const unsigned long zLevelTop = 1; >+ const unsigned long zLevelBottom = 2; >+ const unsigned long zLevelBelow = 3; // below some window >+ >+ /* A window wants to be moved in z-order. calculate whether and how Capital c "Calculate" >+ it should be constrained. Note this method is advisory only: >+ it changes nothing either in WindowMediator's internal state >+ or with the window. >+ >+ @param outPosition constrained position. values like inPosition. Capital v "Values" (or use a comma) >+ @param outBelow if outPosition==zLevelBelow, the window >+ below which inWindow should be placed. Otherwise this >+ this value will be null. >+ @return PR_TRUE if the position returned is different from >+ the position given. >+ */ >+ Line inWindow below up with the rest? >+ boolean calculateZPosition(in nsIXULWindow inWindow, >+ in unsigned long inPosition, >+ in nsIWidget inBelow, >+ out unsigned long outPosition, >+ out nsIWidget outBelow); >+ >Index: appshell/src/nsWindowMediator.cpp >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsWindowMediator.cpp,v >retrieving revision 1.39 >diff -u -r1.39 nsWindowMediator.cpp >--- nsWindowMediator.cpp 2001/09/05 21:28:33 1.39 >+++ nsWindowMediator.cpp 2001/09/17 18:55:18 >@@ -70,9 +70,9 @@ > nsCOMPtr< nsIDOMWindowInternal>& outDOMWindow ); > static nsCOMPtr<nsIDOMNode> GetDOMNodeFromDocShell(nsIDocShell *aShell); > static void GetAttribute( nsIXULWindow* inWindow, >- const nsAutoString& inAttribute, nsAutoString& outValue ); >+ const PRUnichar* inAttribute, nsAutoString& outValue ); Use const nsAString& inAttribute, nsAString& outValue, and line it up with nsIXULWindow on the line above it. > static void GetWindowType( nsIXULWindow* inWindow, nsAutoString& outType ); >-static PRUint32 GetWindowZ( nsIXULWindow *inWindow ); >+static inline PRUint32 GetWindowZ( nsIXULWindow *inWindow ); Instead of this, why not just move the code below up here? >@@ -130,7 +130,7 @@ > } > > >-void GetAttribute( nsIXULWindow* inWindow, const nsAutoString& inAttribute, nsAutoString& outValue ) >+void GetAttribute( nsIXULWindow* inWindow, const PRUnichar* inAttribute, nsAutoString& outValue ) void GetAttribute( nsIXULWindow* inWindow, const nsAString& inAttribute, nsAString& outValue ) > { > nsCOMPtr<nsIDocShell> shell; > if ( inWindow && >@@ -142,33 +142,22 @@ > { > nsCOMPtr<nsIDOMElement> webshellElement( do_QueryInterface(node)); > if ( webshellElement.get() ) >- webshellElement->GetAttribute(inAttribute, outValue ); >+ webshellElement->GetAttribute(nsAutoString(inAttribute), outValue ); And then there's no need for this nsAutoString(). (Just noticed the funky "space before closing parens used throughout this file. Ew!) > } > } > } > > void GetWindowType( nsIXULWindow* inWindow, nsAutoString& outType ) > { >- GetAttribute( inWindow, NS_ConvertASCIItoUCS2("windowtype"), outType ); >+ GetAttribute( inWindow, NS_LITERAL_STRING("windowtype").get(), outType ); And then there's no need for this |.get()| >@@ -500,12 +489,11 @@ > > // Should this title be displayed > PRBool display = PR_TRUE; >- nsAutoString typeAttrib; typeAttrib.AssignWithConversion("intaskslist"); > nsAutoString displayString; >- GetAttribute( inWindow, typeAttrib, displayString ); >+ GetAttribute( inWindow, NS_LITERAL_STRING("intaskslist").get(), displayString ); Nor this |.get()| >@@ -763,7 +753,7 @@ > if( NS_SUCCEEDED ( GetDOMWindow( info->mWindow, DOMWindow ) ) ) > { > *outWindow = DOMWindow; >- (*outWindow)->AddRef(); >+ NS_ADDREF(*outWindow); > } > break; > } Breaking with surrounding indent style. Address these nits and r=jag.

Generally very nice, and thanks especially for cleaning up stuff like nsAutoString as function parameters. Yeesh. However, please don't make all those [noscript] functions scriptable! Every one of those [noscript] functions are for internal bookkeeping; there's no reason any of them should be available to JS. I suppose the proper modern solution would be to move those methods to a nonscriptable, private interface. I wouldn't have any objection to that. But please don't expose them to script.

alright, thanks guys. I'll fix that noscript stuff. I figured it was only noscript because of th parameters. about some of the indenting stuff: excess tabs in this file make my changes look a little wierd. With most editors with 4-space tabs, it looks fine :) About capitalization: yeah yeah, I'll fix it, but all I did was re-wrap the text :)

D'oh! I didn't catch that you were 4-space tabbing it. This file and nsDocShell.cpp will be the only 4-space tabbed files in the whole project now.

Just to be clear: I'm not adding tabs, I'm adding spaces. My editor never inserts tabs. this file is a mix of tabs and spaces, its kinda nasty. There are pleanty of 4-space indented files all over the tree... I'm attaching a new diff -bw patch just so we're all on the same page. I'm not updating the " )" pattern all over this file because it's EVERYWHERE, and I have more important things to fix!

Should've mentioned that it was more a "look at that, ew!" comment than a "while you're there, ..." ;-) So what happened to "when in Rome"? Does that mean I too get to use spaces even if the surrounding code uses tabs?

jag, the "expand tabs when making changes" is a well-known exception to "When in Rome". Another exception: if Rome has been pillaged by Vandals, and there is no prevailing style, assert your own and try to take ownership. You'll be an emperor in no time! /be

I hadn't heard about that exception, but will happily apply it :-)

Great! In the future, consider using CallQueryInterface, e.g. instead of: + if (enumerator) + return enumerator->QueryInterface(NS_GET_IID(nsISimpleEnumerator) , (void**)outEnumerator); do: if (enumerator) return CallQueryInterface(enumerator, outEnumerator); Regardless, sr=waterson

yay, checked in.

Verified changes in 10/19 mozilla nightly build: * Changes in nsIWindowMediator.idl. nsIWindowMediator now is derived from nsISupports, no longer from nsIRDFDataSource. * Changes in nsWindowMediator.h. class nsWindowMediator inherits from public nsIWindowMediator and nsIRDFDataSource. * Changes in nsWindowMediator.cpp. - Made GetWindowZ() inline. - Parameter changes to GetAttribute(). - Use NS_ADDREF(*outWindow); - Added inWindow->GetZlevel(&inZ); - Uses nsAutoString uniqueID(NS_LITERAL_STRING("window-")); - several cosmetic changes . Made