Closed
Bug 123286
Opened 23 years ago
Closed 4 years ago
show warning in Error Console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc)
Categories
(Core :: DOM: Core & HTML, enhancement, P5)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
INACTIVE
mozilla1.2alpha
People
(Reporter: nick, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file, 15 obsolete files)
(deleted),
patch
|
caillon
:
superreview+
|
Details | Diff | Splinter Review |
When denying a window.open() a warning should be sent to the JavaScript console, so that problems can be debugged by power users. Rationale: There are many sites with valid reasons to call window.open() (e.g.: http://www.uol.com.ar/, try to open a chat window). Now it fails with no message (if window.open is forbiden from preferences), if it would have sent a message to the console I wouldn't have spent some time trying to find if it was an evangelism bug. =)
Comment 1•23 years ago
|
||
Um.. there should be a message (uncaught exception, blah, blah, blah). Over to CAPS to investigate.
Assignee: asa → mstoltz
Component: Browser-General → Security: CAPS
QA Contact: doronr → bsharma
Comment 2•23 years ago
|
||
actually... dom.disable_open_during_load seems to not generate any error.... confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•23 years ago
|
||
I don't have time to work on this anytime soon...marking Future.
Severity: normal → enhancement
Status: NEW → ASSIGNED
Summary: There should be a warning when denying a javascript script to open a popup window → [RFE] There should be a warning when denying a javascript script to open a popup window
Target Milestone: --- → Future
Reporter | ||
Comment 4•23 years ago
|
||
I have this working, but without showing the JS line number. I'm working on it.
Reporter | ||
Comment 5•23 years ago
|
||
This is what I have so far. It still lacks i18n and perform some accesses to internal JS structs, but I think it can't be done in other way.
Reporter | ||
Updated•23 years ago
|
Hardware: PC → All
Comment 6•23 years ago
|
||
Comment on attachment 68337 [details] [diff] [review] It works! >+ nsAutoString message; No reason for this. Just use NS_LITERAL_STRING("whatever").get() below. Also, it would be nice to have the message even if you can't get a JSContext and all that.... It won't have line numbers, but it's better than nothing. >+ nsAutoString sourceName; >+ sourceName.AssignWithConversion(fp->script->filename); NS_ConvertUTF8toUCS2 sourceName(fp->script->filename); No reason to set "rv" throughout -- you're returning NS_OK no matter what. And yes, making the string localizable would be good (see nsIStringBundle and nsIStringBundleService)
Attachment #68337 -
Flags: needs-work+
Comment 7•23 years ago
|
||
I think we should add a JS_GetContextFileLine(JSContext *cx, char **fileName, uintN *line); entrypoint to jsdbgapi.h, so that you don't need to include all those private js header files. In general, only jsapi.h and jsdbgapi.h should be included by clients, and only public (JS_*), not private (js_*) entrypoints should be used.
Yeah, I agree. Wanna toss an additional bug up for that?
Reporter | ||
Comment 10•23 years ago
|
||
Yes, I'd like to do that. What you have said is exactly what I think should be done. I'm working on it. BTW, perhaps this logging code should be moved into a utility function used to log errors/warnings to the console. Are there other places where this could be of use? Although it would be a very small function... I don't know...
Reporter | ||
Comment 11•23 years ago
|
||
Uhm.. I now see that it seems that everything could be done with the js debug API, using JS_FrameIterator and friends. It wouldn't hurt to have an utility function to do this somewhere though.
Reporter | ||
Comment 12•23 years ago
|
||
Now it uses only public functions from jsdbg.h. I have also reordered things a little. Still lacks i18n but I think it can be commited (if the patch is ok, of course). PS. I am not allowed to mark the other attachment as obsolete. IMO one shouldn't need a special permission to modify his own attachments.
Comment 13•23 years ago
|
||
Comment on attachment 68627 [details] [diff] [review] Now uses the js debug API psourceName is useless, get rid of it. Indentation here is a little off: + if ( (scripterror = do_CreateInstance("@mozilla.org/scripterror;1", &rv)) && + (consoleService = do_GetService("@mozilla.org/consoleservice;1", &rv))) { The second line of the if's condition should underhang the first clause, I say. Nit of the day! Oops, another nit: + if ( ( cs = do_GetService("@mozilla.org/js/xpc/ContextStack;1", &rv) ) + && (!NS_FAILED(cs->Peek(&cx)))) { This underhangs better, but inconsistently puts the logical connective (&&) on the second line at the front, not at the end of the first line of the condition. I believe jst and I prefer putting that at the end of the first, but he's module owner, so I defer to him. Yet another nit: space after comma in parameter list is not used consistently, e.g. here: + JS_FrameIterator(cx,&fp); Pleaes be consistent, my little mind's hobgoblin thanks you. Another inconsistent style bugaboo: + if(script && pc) Use 'if (' as elsewhere, and in the file's prevailing style. /be
Comment 14•23 years ago
|
||
Comment on attachment 68627 [details] [diff] [review] Now uses the js debug API >+ nsAutoString *psourceName = nsnull, sourceName; What's the point of having psourceName? You never really use it... >+ sourceName.AssignWithConversion(JS_GetScriptFilename(cx,script)); Is this filenam ASCII? Or UTF8? AssignWithConversion assumes ASCII.
Comment 15•23 years ago
|
||
How about an unpopular opinion: I don't see any good reason to figure out the script source name and linenumber here. What value is added that justifies more code? A message to the console seems to me to be reasonable (though not free spacewise). But that message *should* indicate that the window.open was denied because of the *user preference* - the current wording does nothing to make it clear that the user pref had anything to do with the denial. But, I don't see any real value added by calculating and showing which code was denied. Normal users who turn off popups gain nothing - they never look at the console and wouldn't care which code was running. Power users don't gain much either - the mention of the pref is enough to clue them in to the idea that they should turn the pref off if they want to see a popup. The exact location of the window.open call is just fluf and does not justify ading extra code to the app.
Comment 16•23 years ago
|
||
filenames in JS are ASCII, or perhaps ISO-Latin-1 -- they get inflated by zero padding each char when making the filename property of Exception objects. I agree with jband (I just didn't want to be first to be unpopular :-). Let's keep footprint down by choosing not to put in small as well as large patches. Let's try to take code out, where possible. /be
Reporter | ||
Comment 17•23 years ago
|
||
This code is *very* small and it should be taken to a utility function so it can be shared. I can't give reasons to finding the source file and line number, becuse... it seems so obvious to me that this should be done! The goal should be that *every error caused by JavaScript code should be correctly pointed to by the error message*. Again: Every error caused by Javascript code should be correctly labeled, where it happened and why it happened. (IMO =) )
Comment 18•23 years ago
|
||
I'm not so sure I'm convinced. Have you identified other locations where this pattern is useful? The normal error case involves thowing an exception through xpconnect and letting the JS code deal with it or not. XPConnect logs the console error for us automatically. Here it was apparently decided that it would be less disruptive to existing JS web content to just return a null window rather than throwing. So, the console mechanism is missing. Are there really other similar places? Nevertheless, what you want can be done without using the jsapi (much less the jsdbgapi) since we know the call is coming in through xpconnect and xpconnect has facilities for getting the JS caller for us. Note that since we are not yet tracking JS source and linenumbers for event handlers there are plenty of places where we won't have the info yet anyway. Though, as I said, I'm not yet convinced, I coded up an alternative implementation anyway. I'll attach it.
Comment 19•23 years ago
|
||
Comment 20•23 years ago
|
||
Comment on attachment 68827 [details] [diff] [review] An alternative patch that gets the caller info via xpconnect Looks good to me, although scriptError (capital E) would match the consoleService naming convention. I don't think every null return should have a console message with filename and line number, but perhaps this is a hard case -- we've made it harder by adding the dom.disable_open_during_load preference. If this is satisfactory to all parties, I will sr= it. /be
Comment 21•23 years ago
|
||
Hey, I copied nick's names. It's a 'scriptWarning' anyway :) Isn't jst the module owner here? adding him to the cc list. I don't really care if this goes in or not. You guys decide :)
Comment 22•23 years ago
|
||
Hmm, let me restate my foolish position... I agree that emitting a console message is 100% appropriate. My objections to the code for getting the sourcename and lineno are greatly mitigated by how small the code shrank by going through xpconnect and the fact that we are avoiding yet another direct use of js[dbg]api outside of mozilla/js. I would *much* prefer this console message to be localized. AFAICT (on very quick inspection) the DOM does not mess with string bundles. This is perhaps not a good reason to add such a dependency. Given all that, I'd vote for adding this code.
Reporter | ||
Comment 23•23 years ago
|
||
This seems to be settled, so I think John's code should be checked in. I haven't tried it, but it looks much nicer than mine. Well... It's a pitty that it couldn't be my code the one that is checked in. But I surely like the XPCOM approach better (I've even told rginda this on IRC before this patch). In any case it was a good excuse to checkout a mozilla source and see how everything works. Thanks to anyone who has helped me with this intent to do something. =)
Comment 24•23 years ago
|
||
nick: I hope you are not actually discouraged. Even if some of the code you wrote is not going in, this would not be happening at all had you not written the code. There is plenty more to do on this project and we can certainly use the help.
Updated•23 years ago
|
Summary: [RFE] There should be a warning when denying a javascript script to open a popup window → [rfe] show warning in js console when denying a popup window
Reporter | ||
Comment 25•23 years ago
|
||
Hi! I just wanted to note that the fix for bug 117707 makes this code useable in many places. With that fix, all those settings cause Mozilla to fail without explaining a thing to the user.
Reporter | ||
Updated•23 years ago
|
Summary: [rfe] show warning in js console when denying a popup window → [rfe] show warning in js console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc)
Reporter | ||
Comment 26•23 years ago
|
||
Building on John Bandhauer I've created another patch. I've moved the code to its own function and added calls to it from everywhere.
Reporter | ||
Comment 27•23 years ago
|
||
Updated•23 years ago
|
Attachment #68337 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #68627 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #68827 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #80261 -
Attachment is obsolete: true
Comment 28•23 years ago
|
||
Comment on attachment 82177 [details] [diff] [review] New version of patch. >Index: nsGlobalWindow.cpp >=================================================================== >RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v >retrieving revision 1.516 >diff -u -r1.516 nsGlobalWindow.cpp >--- nsGlobalWindow.cpp 27 Apr 2002 00:03:47 -0000 1.516 >+++ nsGlobalWindow.cpp 3 May 2002 07:45:16 -0000 >@@ -143,6 +143,9 @@ > #include "nsIBindingManager.h" > #include "nsIXBLService.h" > >+// For logging messages to the console >+#include "nsIScriptError.h" >+#include "nsIConsoleService.h" > > static nsIEntropyCollector* gEntropyCollector = nsnull; > static PRInt32 gRefCnt = 0; >@@ -214,6 +217,35 @@ > return !prefValue; > } > >+static void sendWarningMessage(nsIXPConnect* sXPConnect, nsAFlatString& msg) >+{ >+ nsCOMPtr<nsIScriptError> >+ scriptError(do_CreateInstance("@mozilla.org/scripterror;1")); >+ nsCOMPtr<nsIConsoleService> >+ consoleService(do_GetService("@mozilla.org/consoleservice;1")); >+ >+ if (scriptError && consoleService) { >+ PRInt32 lineno = 0; Don't you want an unsigned int? PRUint32? Also, please use interCaps for the varname. lineNum might make more sense. >+ nsAutoString sourceName; >+ nsCOMPtr<nsIStackFrame> frame; >+ sXPConnect->GetCurrentJSStack(getter_AddRefs(frame)); >+ if (frame) { >+ frame->GetLineNumber(&lineno); >+ nsXPIDLCString fileName; >+ if (NS_SUCCEEDED(frame->GetFilename(getter_Copies(fileName)))) { >+ sourceName.AssignWithConversion(fileName); avoid AssignWithConversion in favor of NS_LITERAL_STRING, eg sourceName.Assign(NS_LITERAL_CSTRING(fileName)); >+ } >+ } >+ >+ if (NS_SUCCEEDED(scriptError->Init(msg.get(), sourceName.get(), >+ nsnull, (uintN)lineno, And as a result of using a PRUint32 above, you can kill the cast here. >+ 0, nsIScriptError::warningFlag, >+ "content javascript"))) { >+ consoleService->LogMessage(scriptError); >+ } >+ } >+} >+ <...> > if (!CanSetProperty("dom.disable_window_status_change") && !IsCallerChrome()) { >+ static NS_NAMED_LITERAL_STRING(msg, >+ "Attempt to change window status message denied due to user preference"); We're in the JS console where users expect some code output. So, please mention window.status in your message. Something like "Setting window.status denied due to..." >+ sendWarningMessage(sXPConnect,msg); Please put a space between arguments here and the same below. > return NS_OK; > } > >@@ -1389,6 +1424,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_status_change") && !IsCallerChrome()) { >+ static NS_NAMED_LITERAL_STRING(msg, >+ "Attempt to change window default status message denied due to user preference"); Mention window.defaultStatus similar to above. >+ sendWarningMessage(sXPConnect,msg); > return NS_OK; > } > >@@ -1448,6 +1486,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ static NS_NAMED_LITERAL_STRING(msg, >+ "window.setInnerWidth() denied due to user preference"); I'd like to prepend "Calling " on to the messages here (and below). eg, "Calling window.foo() denied...." >+ sendWarningMessage(sXPConnect,msg); > return NS_OK; > } > <...> >@@ -2864,6 +2938,9 @@ > #ifdef DEBUG > printf ("*** Blocking window.open.\n"); > #endif Should we remove the ifdef above? We'll have two messages output in debug mode (one for the printf here, and one for the JS warning)... >+ static NS_NAMED_LITERAL_STRING(msg, >+ "window.open denied due to user preference"); >+ sendWarningMessage(sXPConnect,msg); > *_retval = nsnull; > return NS_OK; > }
Attachment #82177 -
Flags: needs-work+
Comment 29•23 years ago
|
||
> sourceName.Assign(NS_LITERAL_CSTRING(fileName));
That would _so_ not work. fileName is not a literal string, after all...
My original question stands however. Is fileName ASCII? UTF-8?
Platform-encoded? The current code assumes it's ASCII.
Reporter | ||
Comment 30•23 years ago
|
||
>>+ PRInt32 lineno = 0;
> Don't you want an unsigned int? PRUint32?
getFileNumner() expects this.
I have applied everything else.
bz: I've traced the filename. For JavaScipt it seems to be an opaque item.
I've ran Mozilla in gdb and disocvered who called JS_Evaluate*(). I've found
out
that "filename" gets assigned from url.get(). So? Is that just ASCII?
Comment 31•23 years ago
|
||
That's almost certainly UTF8, then. So you want to use: sourceName = NS_ConvertUTF8toUCS2(fileName);
Reporter | ||
Comment 32•23 years ago
|
||
Uhm.. but wouldn't an URL be already "URL-encoded"? I guess this is just plain ASCII always.
Reporter | ||
Comment 34•23 years ago
|
||
Comment 35•23 years ago
|
||
Re-assigning to nick who is working on this. Sorry for being hasty earlier with my first review. Anyway, I am against this patch in general because of not many people turning on strict warnings, its not really even in the UI for some of our browsers. It just seems to be extra fluff that we don't really need. Those users who are smart enough to turn on strict warnings probably don't need to be told why scripts are breaking if applicable. The JS console sucks down enough memory with real warnings. I don't think that adding fake warnings to remind me that I blocked setting window.status is necessary at all. Were it up to me, this would be a WONTFIX bug. Of course, this is all IMO. That said, the patch looks fine to me and r=caillon if you need it. However, I would prefer if you got jband to do the r= since your work is built in part on some of his work (and vice versa). Additionaly, I would like to see this question addressed: Do we really want to warn for *all* of these items? Warning for window.status as an example seems a bit overboard. Back when this bug was filed, setting window.status while the preference blocked it broke scripts because we used CAPS, but with my checkin to bug 117707, they are now silently ignored. I think that since the only one of these preferences which truly break scripts today is the window.open one, then that should be the only one we warn about. When I block something, I expect it to be silently ignored everywhere. I don't want to be reminded in my console everytime someone tries to set window.status. (For example those sites with scrolling window.statuses would be unbearable both in my debug console and my JS console)...
Assignee: mstoltz → nick
Status: ASSIGNED → NEW
Comment 36•23 years ago
|
||
Comment on attachment 82725 [details] [diff] [review] Now with NS_ConvertUTF8toUCS2! +static void sendWarningMessage(nsIXPConnect* sXPConnect, nsAFlatString& msg) Make this a static member function of GlobalWindowImpl (and uppercase the first character in the name), that way you don't need to pass in sXPConnect all the time... >+ static NS_NAMED_LITERAL_STRING(msg, >+ "Setting window.status denied due to user preference"); I don't see a need for those named literals to be static, no need for those literal string wrappers to remain live after they're used... Remove the static keyword from all of these... Change that and I'll sr.
Attachment #82725 -
Flags: needs-work+
Reporter | ||
Comment 37•23 years ago
|
||
Reply to Christopher:
Stricts warnings? This warnings would be shown to everybody. I haven't checked
the "show JS strict warnings" myself.
I agree that the window.open is the only one that could break scripts, and it's
the one that provide the justification. But adding a warning to the other ones
id free, all the "bloat" has already happened.
Besides, as I said earlier, Mozilla will be used by millions ( =)! ) and we'll
get a lot of bug reports of users who claim that Mozilla is broken, only because
they activated this preference (or a friend did so for them when he installed
mozilla).
> When I block something, I expect it to be silently ignored everywhere. I
> don't want to be reminded in my console everytime someone tries to set
> window.status.
It's like logging to syslog when somebody tries to break into your computer.
Besides I compare this to accessing window.event, Mozilla logs that every time
and if the code is in a mouse handler you'll see it repeated. However perhaps
something could be done in the future to log once per window (per message type).
I don't know, but anyway I would commit this and make any tidyness work later...
What do you think?
To jst:
Oops.. I didn't realize it was a *static* member. I could make this a member
function. I had thought that nothing was gained since the this pointer would
have needed to be passed anyway, but if it's static member...
Reporter | ||
Comment 38•23 years ago
|
||
This is it. But I have a doubt.. couldn't I replace those NS_NAMED_LITERAL_STRING(msg,"...") and just use NS_LITERAL_STRING("...") inside the function call? PS: Ugh.. the bug is assigned to me and I'm still not authorized to obsolete my own attachments.. :(
Comment 39•23 years ago
|
||
Comment on attachment 82771 [details] [diff] [review] Now SendWarning is a static member of the class. Yes, you could do that, but this is IMO a bit cleaner, and doesn't cost any more... sr=jst
Attachment #82771 -
Flags: superreview+
Comment 40•23 years ago
|
||
I agree with caillon that changing window.status should not put a warning in the js console. The most common use for changing window.status is in mouseover events and the console would fill up quickly. Besides, it would be silly to put a warning in the status bar (bug 47128) just to tell the user that a script has been denied permission to write text in the status bar. The only prefs that should create warnings are opening windows and maybe resizing windows. Focusing windows, changing status bar text, and swapping images aren't important enough that we need to grab the user's attention, and they're called so often that the console would fill up quickly (mlk). The cookie prefs shouldn't even be there -- scripts should be allowed to store cookies if and only if the server is allowed to store cookies. The error message for opening a window should include the URL that the site was trying to open. Clicking the link in the console should open it with the correct referrer and security domain. (I think the source-code links in the console already get this right.) The link should obey the "re-use browser windows for links opened from other apps" pref (bug 75138), but for now I don't care whether it opens a new window or re-uses the most recent browser window.
Reporter | ||
Comment 41•23 years ago
|
||
I agree with almost everything you've just said. I will modify the patch. But I don't think I will be able to implement the "open the window as it would have been opened by the script". I am not an expert but that would probably need modifications to the JS console. And.. besides it would be a little inconsistent. The JS console shows issues in JS code, I wouldn't expect it to open a content window.
Status: NEW → ASSIGNED
Updated•23 years ago
|
Attachment #82279 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #82725 -
Attachment is obsolete: true
Reporter | ||
Comment 43•23 years ago
|
||
Would it be ok if I added a member variable PRUint32 mSomeBits (to be used to miscelaneous bits =) ) to the GlobalWindowImpl class? (With a typedef enum { miscBitAlreadyShownWindoStatusMessgae = 1 << 0 }) This bit would be used to show the window.status message just once per document loaded. I would clear that bit in GlobalWindowImpl::SetNewDocument().
Comment 44•23 years ago
|
||
Do we really need to go there? I don't think this is important enough, either show all these messages always, or don't show them at all. Just my thoughts...
Reporter | ||
Comment 45•23 years ago
|
||
What about doing something like this? static GlobalWindowImpl *last = NULL; if(last==NULL || this!=last) { NS_NAMED_LITERAL_STRING(msg, "Setting window.status denied due to user preference"); last=this; } ? It would log the first time for each window, with the weird side effect that logging for a window "clears the flag" for other windows. But the hack would be self contained.
Reporter | ||
Comment 46•23 years ago
|
||
I've removed the logging of window.status by now, I've added the logging of parameters. I've tested this and it's very nice seeing in the console where did a server wanted to take us with window.open(). =) I have added i18n with a helper function, which is a generalization of a current function (makeScriptDialogTitle), so this won't add any extra bloat. Now I must make the other code use my generalized function.
Attachment #82177 -
Attachment is obsolete: true
Attachment #82771 -
Attachment is obsolete: true
Comment 47•23 years ago
|
||
Have an icon on the bottom taskbar of the Browser, when Mozilla kills a pop-up it will show a tiny icon of a bubble bursting, or a baloon popping - something to show it's working. It can also show new users (desktop) how good Mozilla is at keeping those pop-ups under control. Additionally, it would be nice if you could find out the window it killed. Maybe a log of some sort? And maybe a "pop" sound when it kills a pop-up, along with the animation. This entire feature should be modular, though. I mean, you should be able to disable it in the preferences area.
Reporter | ||
Comment 48•23 years ago
|
||
Please, somebody review this version and (hopefuly) check it in. Thanks.
Attachment #84193 -
Attachment is obsolete: true
Comment 49•23 years ago
|
||
Comment on attachment 86123 [details] [diff] [review] New version +static nsAFlatString &getLocalizedMessage(nsAFlatString &out, const nsAFlatString &name, + const nsAFlatString *in1 = nsnull, + const nsAFlatString *in2 = nsnull) Make the next line arguments line up with the first argument on the first line, and please split the first like so that the return type is on its own line. +{ + out.Truncate(0); + nsresult rv; + nsCOMPtr<nsIStringBundleService> stringBundleService = + do_GetService(kCStringBundleServiceCID, &rv); + + if (NS_SUCCEEDED(rv) && stringBundleService) { No need to get the rv from do_GetService() and no need to check it, just check for stringBundleService being non-null, anything else is just redundant. + nsCOMPtr<nsIStringBundle> stringBundle; + rv = stringBundleService->CreateBundle(kDOMBundleURL, + getter_AddRefs(stringBundle)); Since rv is not used, why assign into it? + if (stringBundle) { + int num; + nsXPIDLString tempString; + const PRUnichar *formatStrings[2]; + if (in1!=nsnull) { + formatStrings[0] = in1->get(); + if (in2!=nsnull) { Don't do "if (ptr!=nsnull)", simply "if (ptr)" is more readable. + formatStrings[1] = in2->get(); + num=2; + } else + num=1; + } else + num=0; + rv = stringBundle->FormatStringFromName(name.get(), + formatStrings, num, getter_Copies(tempString)); + if (tempString) + out.Assign(tempString.get()); Loose the .get() here, that only makes this operation more expensive. +static inline nsAFlatString &getLocalizedMessage(nsAFlatString &out, const nsAFlatString &name, + PRInt32 anInt) Again, make sure the arguments line up... +{ + nsAutoString anIntString; + anIntString.AppendInt(anInt); + return getLocalizedMessage(out, name, &anIntString); +} Does this really need to be inline? And only indent the code 2 spaces, no tabs!!! + +static inline nsAFlatString &getLocalizedMessage(nsAFlatString &out, const nsAFlatString &name, + PRInt32 anInt, PRInt32 anotherInt) Same here, argument indentation, and does this really need to be inline? +{ + nsAutoString anIntString, anotherIntString; + anIntString.AppendInt(anInt); + anotherIntString.AppendInt(anotherInt); + return getLocalizedMessage(out, name, &anIntString, &anotherIntString); +} Again, 2 space indentation, no tabs!!! +void GlobalWindowImpl::SendWarningMessage(const nsAFlatString& msg) Put the return type on its own line... Change that and I'll have one more look...
Attachment #86123 -
Flags: needs-work+
Reporter | ||
Updated•23 years ago
|
Attachment #86123 -
Attachment is obsolete: true
Comment 51•23 years ago
|
||
Comment on attachment 86563 [details] [diff] [review] Tidier version sr=jst
Attachment #86563 -
Flags: superreview+
Comment 52•23 years ago
|
||
Comment on attachment 86563 [details] [diff] [review] Tidier version >+ stringBundleService->CreateBundle(kDOMBundleURL, >+ getter_AddRefs(stringBundle)); This indentation is odd; the "getter" should line up with "kDOMBundleURL", no? >+ nsAutoString msg, status(aDefaultStatus); >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowDefaultStatusDenied"), >+ &status)); use PromiseFlatString() instead of the |status| temporary? >+ nsAutoString aFlatTitle(aTitle); >+ getLocalizedMessage(title, NS_LITERAL_STRING("ScriptDlgTitle"), &aFlatTitle); again, use PromiseFlatString() >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowMoveByDenied"),aXDif,aYDif)); spaces after those commas before aXdif, aYdif? >+ "WindowResizeToDenied"),aWidth,aHeight)); spaces? >+ "WindowResizeByDenied"),aWidthDif,aHeightDif)); spaces? >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowOpenDenied"),&url)); space before "&url"? Fix those and r=bzbarsky
Comment 53•23 years ago
|
||
Comment on attachment 86563 [details] [diff] [review] Tidier version I agree with bz's issues but I have a few more things to add: >Index: dom/src/base/nsGlobalWindow.cpp >=================================================================== >RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v > >+static nsAFlatString& >+getLocalizedMessage(nsAFlatString &out, >+ const nsAFlatString &name, >+ const nsAFlatString *in1 = nsnull, >+ const nsAFlatString *in2 = nsnull) Please use more descriptive names for the arguments. Use aParam as the style for the argument name. Also the out parameter should be the last parameter. Just make sure you adjust any code for nsnull in params (which it appears you're already checking for so you should be fine). Also, why do you even have an outparameter if you are just returning it? That is redundant. In this case, I would suggest returning an nsresult (or even void I guess would be fine), keep the out parameter and not inlining your functions which will reduce the long lines in other parts of the code. static nsresult getLocalizedMessage(const nsAFlatString *aName, const nsAFlatString *aString1, const nsAFlatString *aString2, nsAFlatString &aResult); >+{ >+ out.Truncate(0); >+ nsCOMPtr<nsIStringBundleService> stringBundleService = >+ do_GetService(kCStringBundleServiceCID); >+ >+ if (stringBundleService) { >+ nsCOMPtr<nsIStringBundle> stringBundle; >+ stringBundleService->CreateBundle(kDOMBundleURL, >+ getter_AddRefs(stringBundle)); >+ >+ if (stringBundle) { >+ int num; >+ nsXPIDLString tempString; >+ const PRUnichar *formatStrings[2]; >+ if (in1) { >+ formatStrings[0] = in1->get(); >+ if (in2) { >+ formatStrings[1] = in2->get(); >+ num=2; >+ } else >+ num=1; >+ } else >+ num=0; I find the |} else foo| hard to read. If you have brackets in the if, then also use brackets on the else. (Yes I know this an uber-nit, but I've had to pick nits like this from sr=s in the past and I'm just passing it on since I agree). >+ stringBundle->FormatStringFromName(name.get(), >+ formatStrings, num, getter_Copies(tempString)); Please indent the arguments so they are below the first one, eg: stringBundle->FormatStringFromName(name.get(), formatStrings, num, getter_Copies(tempString)); >+ if (tempString) >+ out.Assign(tempString); >+ } >+ } >+ >+ // Just in case >+ if (out.IsEmpty()) { >+ NS_WARNING("could not get string from string bundle"); >+ out=NS_LITERAL_STRING("<unknown>"); out.Assign() ? >+ } >+ return out; >+} >+ >+static nsAFlatString& >+getLocalizedMessage(nsAFlatString &out, const nsAFlatString &name, >+ PRInt32 anInt) Again some better param names, move the out param to the end, and if you can't fit all the params on one line, then stack them under each other. Same for all other places below. <...> >+ >+void GlobalWindowImpl::SendWarningMessage(const nsAFlatString& msg) >+{ >+ NS_ASSERTION(msg.get(), "SendWarningMessage() called with msg.get() == NULL"); >+ nsCOMPtr<nsIScriptError> >+ scriptError(do_CreateInstance(kScriptErrorContractID)); >+ nsCOMPtr<nsIConsoleService> >+ consoleService(do_GetService(kConsoleServiceContractID)); >+ >+ if (scriptError && consoleService) { >+ PRInt32 lineNumber = 0; >+ nsAutoString sourceName; >+ nsCOMPtr<nsIStackFrame> frame; >+ sXPConnect->GetCurrentJSStack(getter_AddRefs(frame)); >+ if (frame) { >+ frame->GetLineNumber(&lineNumber); >+ nsXPIDLCString fileName; >+ if (NS_SUCCEEDED(frame->GetFilename(getter_Copies(fileName)))) { >+ sourceName = NS_ConvertUTF8toUCS2(fileName); >+ } >+ } >+ >+ if (NS_SUCCEEDED(scriptError->Init(msg.get(), sourceName.get(), >+ nsnull, (uintN)lineNumber, >+ 0, nsIScriptError::warningFlag, >+ "content javascript"))) { >+ consoleService->LogMessage(scriptError); >+ } >+ } >+} >+ > //***************************************************************************** > //*** GlobalWindowImpl: Object Management > //***************************************************************************** >@@ -1368,6 +1465,10 @@ > */ > > if (!CanSetProperty("dom.disable_window_status_change") && !IsCallerChrome()) { >+ nsAutoString msg, status(aDefaultStatus); >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowDefaultStatusDenied"), Ack. At first glance this looks like "WindowDefaultStatusDenied" is a string parameter by itself to getLocalizedMessage() because of your indentation. If you follow my suggestion above, then yuo can do getLocalizedMessage(msg, NS_LITERAL_STRING("WindowDefaultStatusDenied"), &status); SendWarningMessage(status); which avoids so much happening all on one line... >+ &status)); > return NS_OK; > } > >@@ -1428,6 +1529,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowSetInnerWidthDenied"),aInnerWidth)); Same as above. Fix this anywhere else it happens in the code. > return NS_OK; > } > <...> >@@ -2151,7 +2232,8 @@ > const PRUnichar *title = nsnull; > nsresult rv = CheckSecurityIsChromeCaller(&isChrome); > if (NS_FAILED(rv) || !isChrome) { >- MakeScriptDialogTitle(NS_LITERAL_STRING(""), newTitle); >+ NS_NAMED_LITERAL_STRING(emptyString, ""); >+ getLocalizedMessage(newTitle,NS_LITERAL_STRING("ScriptDlgTitle"), &emptyString); > title = newTitle.get(); > } > NS_WARN_IF_FALSE(!isChrome, "chrome shouldn't be calling alert(), use the prompt service"); >@@ -2182,7 +2264,8 @@ > const PRUnichar *title = nsnull; > nsresult rv = CheckSecurityIsChromeCaller(&isChrome); > if (NS_FAILED(rv) || !isChrome) { >- MakeScriptDialogTitle(NS_LITERAL_STRING(""), newTitle); >+ NS_NAMED_LITERAL_STRING(emptyString, ""); >+ getLocalizedMessage(newTitle, NS_LITERAL_STRING("ScriptDlgTitle"), &emptyString); > title = newTitle.get(); > } > NS_WARN_IF_FALSE(!isChrome, "chrome shouldn't be calling confirm(), use the prompt service"); >@@ -2226,7 +2309,8 @@ > PRBool isChrome = PR_FALSE; > rv = CheckSecurityIsChromeCaller(&isChrome); > if (NS_FAILED(rv) || !isChrome) { >- MakeScriptDialogTitle(aTitle, title); >+ nsAutoString aFlatTitle(aTitle); >+ getLocalizedMessage(title, NS_LITERAL_STRING("ScriptDlgTitle"), &aFlatTitle); > } > else > { >@@ -2328,6 +2412,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_flip") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowFocusDenied"))); > return NS_OK; > } > >@@ -2473,6 +2560,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowMoveToDenied"),aXPos,aYPos)); > return NS_OK; > } > >@@ -2498,6 +2588,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowMoveByDenied"),aXDif,aYDif)); > return NS_OK; > } > >@@ -2527,6 +2620,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowResizeToDenied"),aWidth,aHeight)); > return NS_OK; > } > >@@ -2552,6 +2648,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowResizeByDenied"),aWidthDif,aHeightDif)); > return NS_OK; > } > >@@ -2582,6 +2681,9 @@ > */ > > if (!CanSetProperty("dom.disable_window_move_resize") && !IsCallerChrome()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowSizeToContentDenied"))); > return NS_OK; > } > >@@ -2851,17 +2953,6 @@ > NS_ENSURE_STATE(sXPConnect); > nsresult rv = NS_OK; > >- /* If we're in a commonly abused state (top level script, running a timeout, >- * or onload/onunload), and the preference is enabled, block the window.open(). >- */ >- if (CheckForAbusePoint()) { >-#ifdef DEBUG >- printf ("*** Blocking window.open.\n"); >-#endif >- *_retval = nsnull; >- return NS_OK; >- } >- > nsCOMPtr<nsIXPCNativeCallContext> ncc; > > rv = sXPConnect->GetCurrentNativeCallContext(getter_AddRefs(ncc)); >@@ -2893,6 +2984,17 @@ > nsJSUtils::ConvertJSValToString(options, cx, argv[2]); > } > } >+ } >+ >+ /* If we're in a commonly abused state (top level script, running a timeout, >+ * or onload/onunload), and the preference is enabled, block the window.open(). >+ */ >+ if (CheckForAbusePoint()) { >+ nsAutoString msg; >+ SendWarningMessage(getLocalizedMessage(msg, NS_LITERAL_STRING( >+ "WindowOpenDenied"),&url)); >+ *_retval = nsnull; >+ return NS_OK; > } > > return OpenInternal(url, name, options, PR_FALSE, nsnull, 0, nsnull, >Index: dom/src/base/nsGlobalWindow.h >=================================================================== >RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.h,v >retrieving revision 1.187 >diff -u -r1.187 nsGlobalWindow.h >--- dom/src/base/nsGlobalWindow.h 22 May 2002 00:39:21 -0000 1.187 >+++ dom/src/base/nsGlobalWindow.h 6 Jun 2002 06:42:13 -0000 >@@ -257,6 +257,8 @@ > PRBool searchInFrames, PRBool showDialog, > PRBool *aReturn); > >+ static void SendWarningMessage(const nsAFlatString& msg); >+ > protected: > // When adding new member variables, be careful not to create cycles > // through JavaScript. If there is any chance that a member variable
Attachment #86563 -
Flags: needs-work+
Reporter | ||
Comment 54•23 years ago
|
||
caillon: I agree with almost nothing of your review. Anyway, I've started to do the changes out of tiredness (I won't move the out parameter to be first, that doesn't make sense in this printf wannabe). I'll do the rest even if I don't agree, I'll send a patch tommorrow.
Target Milestone: Future → mozilla1.2alpha
Reporter | ||
Comment 55•23 years ago
|
||
Well, I've done all of bz suggestions and some of caillon ones. I've havent done these things: 1) I haven't put the out argument last. This is supposed to be a function similar to sprintf, where you have a variable list of arguments at the end. 2) I still return the out value. The idea of a "getTranslatedMessage" IMO is get out of the view of the casual reader. See how gettext is used in GNU programs. I hope this is ok. Please, sbdy sr= (again =( ) and r= this.
Attachment #86563 -
Attachment is obsolete: true
Comment 56•23 years ago
|
||
Comment on attachment 91304 [details] [diff] [review] Tidiest version. >+ if (in1) { >+ formatStrings[0] = in1->get(); >+ if (in2) { >+ formatStrings[1] = in2->get(); >+ num=2; >+ } else { >+ num=1; >+ } >+ } else { >+ num=0; >+ } This is assuming that you don't get a null |in1| with a non-null |in2|. Please add an assertion to that effect (NS_PRECONDITION) to the beginning of your function to catch possible bad callers in the future. >+ if (tempString) >+ out.Assign(tempString); Could we make that check be |if (!tempString.IsEmpty())| ? >+ nsAutoString aFlatTitle(aTitle); >+ getLocalizedMessage(title, NS_LITERAL_STRING("ScriptDlgTitle"), &aFlatTitle); Like I said in comment 52, use PromiseFlatString here. Fix those (the first one is _very_ important, the other two I'd like to see fixed as well) and r/sr=bzbarsky
Comment 57•23 years ago
|
||
At the least could you use the "aArgument" style for your function params? Just because there is existing code that doesn't follow our style guidelines is no reason to introduce new code that breaks them... And I still think the names of your variables are completely useless, but at the least with using aIn1, aIn2, you'll be able to tell at a glance that it's a parameter, and not just a local variable...
Reporter | ||
Comment 58•23 years ago
|
||
Now it uses PromiseFlatString (I had to store it somewhere because I can't take the address of a temporary). I've renamed in1 and in2 to aFirstParam and aSecondParam. (My own naming logic is: short parameter names for short scope variables and long descriptive for class member or global symbols. That's why I would have used just in1 and in2.)
Attachment #91304 -
Attachment is obsolete: true
Comment 59•23 years ago
|
||
Two things: 1) Why can't you take the address of a temporary exactly? 2) If you really can't: const nsAString & flatTitle = PromiseFlatString(aTitle); This using a reference instead of creating another object with a copy constructor, and not using a "aFoo" pattern (reserved for arguments) for a local stack variable.
Reporter | ||
Comment 60•23 years ago
|
||
Uhm... PromiseFlatString returns an object by value. After this function has returned flatTitle will be a reference to what? Will it be constructed in the stack and destroyed when the function ends? I see this is the way is being done in other parts of the code, but I don't quite understand what's happening. Ok, I'll trust you and I'll do it =) (I'll use nsAFlatString& as the target type though). Ah! I thought the "a" was an article, like.. aParameter, anOrange, anObject... like it's seen in some OO languages. =)
Attachment #91447 -
Attachment is obsolete: true
Comment 61•23 years ago
|
||
Comment on attachment 91509 [details] [diff] [review] Correctly flattened strings r/sr=bzbarsky The reference is a reference to the object that was returned; the object will be destroyed once all references go out of scope.
Attachment #91509 -
Flags: superreview+
Reporter | ||
Updated•23 years ago
|
Attachment #91509 -
Attachment is obsolete: true
Comment 64•23 years ago
|
||
Comment on attachment 92314 [details] [diff] [review] Indentation So it looks like most of my nits I gave nick via IRC were fixed. A few aren't, and that's just as well because I have a few more to dish out. >Index: dom/src/base/nsGlobalWindow.cpp >=================================================================== >RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v >retrieving revision 1.533 >diff -u -r1.533 nsGlobalWindow.cpp >--- dom/src/base/nsGlobalWindow.cpp 10 Jul 2002 04:58:59 -0000 1.533 >+++ dom/src/base/nsGlobalWindow.cpp 23 Jul 2002 01:25:58 -0000 ... >@@ -169,6 +172,8 @@ > > static const char *kDOMBundleURL = "chrome://global/locale/commonDialogs.properties"; > >+static const char *kConsoleServiceContractID = NS_CONSOLESERVICE_CONTRACTID; >+static const char *kScriptErrorContractID = NS_SCRIPTERROR_CONTRACTID; > Please move those 2 new contract IDs down with the other contract IDs right below it. Fix indentation so the = operators align (modifying the others' indentation if needed) > > static const char * const kCryptoContractID = NS_CRYPTO_CONTRACTID; >@@ -188,6 +193,102 @@ > return !prefValue; > } > >+static nsAFlatString& >+getLocalizedMessage(nsAFlatString &aOut, >+ const nsAString &aName, >+ const nsAFlatString *aString1 = nsnull, >+ const nsAFlatString *aString2 = nsnull) >+{ >+ aOut.Truncate(); >+ nsCOMPtr<nsIStringBundleService> stringBundleService = >+ do_GetService(kCStringBundleServiceCID); >+ >+ if (stringBundleService) { >+ nsCOMPtr<nsIStringBundle> stringBundle; >+ stringBundleService->CreateBundle(kDOMBundleURL, >+ getter_AddRefs(stringBundle)); >+ >+ if (stringBundle) { >+ PRUint32 num; >+ nsXPIDLString tempString; >+ const PRUnichar *formatStrings[2]; >+ if (aString1) { >+ formatStrings[0] = aString1->get(); >+ if (aString2) { >+ formatStrings[1] = aString2->get(); >+ num=2; Please add spaces between the operands and the operator. |num = 2;|. Do the same for the other instances in this function. >+ } else { >+ num=1; >+ } >+ } else { >+ NS_ASSERTION(aString2==nsnull, "getLocalizedMessage() string1 == NULL && string2 != NULL"); >+ num=0; >+ } >+ stringBundle->FormatStringFromName(PromiseFlatString(aName).get(), >+ formatStrings, num, >+ getter_Copies(tempString)); >+ if (!tempString.IsEmpty()) >+ aOut.Assign(tempString); >+ } >+ } >+ Don't add a line of just whitespace. The extra newline is fine, just make sure there aren't any spaces there. >+ // Just in case >+ if (aOut.IsEmpty()) { >+ NS_WARNING("could not get string from string bundle"); >+ aOut.Assign(NS_LITERAL_STRING("<unknown>")); >+ } >+ return aOut; >+} >+ >+static nsAFlatString& >+getLocalizedMessage(nsAFlatString &aOut, const nsAString &aName, This is extremely redundant and unncessary. Don't return a value and also pass it as an out parameter. In subsequent code you are doing: if (foo) { nsAutoString bar; getLocalizedMessage(bar, baz, bat); } Please pick either an out parameter and returning an nsresult or just return the string. Also, after you do that, please put each parameter on separate lines. >+ PRInt32 aInt) >+{ >+ nsAutoString string; >+ string.AppendInt(aInt); >+ return getLocalizedMessage(aOut, aName, &string); >+} >+ >+static nsAFlatString& >+getLocalizedMessage(nsAFlatString &aOut, const nsAString &aName, >+ PRInt32 aInt1, PRInt32 aInt2) Parameters on separate lines as above. E.G. static nsAFlatString& getLocalizedMessage(nsAFlatString &aOut, const nsAString &aName, PRInt32 aInt1, PRInt32 aInt2) >+{ >+ nsAutoString string1, string2; >+ string1.AppendInt(aInt1); >+ string2.AppendInt(aInt2); >+ return getLocalizedMessage(aOut, aName, &string1, &string2); >+} >+ >+void GlobalWindowImpl::SendWarningMessage(const nsAFlatString& aMessage) Please put the void on a line by itself. void GlobalWindowImpl::.... >+{ >+ NS_ASSERTION(aMessage.get(), "SendWarningMessage() called with aMessage.get() == NULL"); >+ nsCOMPtr<nsIScriptError> >+ scriptError(do_CreateInstance(kScriptErrorContractID)); We use 2 space indent in this file. Not 4. >+ nsCOMPtr<nsIConsoleService> >+ consoleService(do_GetService(kConsoleServiceContractID)); >+ Same as above, plus you have a line of just whitespace. Remove the extra space chars please. >+ if (scriptError && consoleService) { >+ PRInt32 lineNumber = 0; >+ nsAutoString sourceName; >+ nsCOMPtr<nsIStackFrame> frame; >+ sXPConnect->GetCurrentJSStack(getter_AddRefs(frame)); >+ if (frame) { >+ frame->GetLineNumber(&lineNumber); >+ nsXPIDLCString fileName; >+ if (NS_SUCCEEDED(frame->GetFilename(getter_Copies(fileName)))) { >+ sourceName = NS_ConvertUTF8toUCS2(fileName); >+ } >+ } >+ >+ if (NS_SUCCEEDED(scriptError->Init(aMessage.get(), sourceName.get(), >+ nsnull, (uintN)lineNumber, >+ 0, nsIScriptError::warningFlag, >+ "content javascript"))) { I bypassed this my first time around, but I won't this time. The arguments are hard to read like this. Please break each arg up onto a separate line and please comment what 0 means here... if (...Init(aMessage.get(), sourceName.get(), nsnull, (uintN)lineNumber, 0, // column 0 nsIScriptError::warningFlag, "content javascript"))) { >+ consoleService->LogMessage(scriptError); >+ } >+ } >+} >+ > //***************************************************************************** > //*** GlobalWindowImpl: Object Management > //***************************************************************************** >@@ -1367,6 +1468,11 @@ > */ > > if (!CanSetProperty("dom.disable_window_status_change") && !IsCallerChrome()) { >+ nsAutoString msg; Argh, useless var because of my earlier comment. And so on through out the patch...
Attachment #92314 -
Flags: superreview+
Attachment #92314 -
Flags: needs-work+
Updated•21 years ago
|
Component: Security: CAPS → DOM
Summary: [rfe] show warning in js console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc) → [rfe] show warning in Error Console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc)
Updated•17 years ago
|
Assignee: nick → nobody
Status: ASSIGNED → NEW
QA Contact: ian → general
Summary: [rfe] show warning in Error Console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc) → show warning in Error Console when denying from "scripts & windows" a popup window, or other javascript functions (resizeTo,focus,etc)
Updated•10 years ago
|
Flags: needinfo?(nick)
Updated•10 years ago
|
Flags: needinfo?(nick)
Comment 67•10 years ago
|
||
[Tracking Requested - why for this release]: [Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0M?
tracking-b2g:
--- → ?
Updated•10 years ago
|
blocking-b2g: 2.0M? → 2.1S?
Updated•10 years ago
|
blocking-b2g: 2.1S? → ---
tracking-b2g:
? → ---
Comment 68•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that havenβt been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Updated•4 years ago
|
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INACTIVE
You need to log in
before you can comment on or make changes to this bug.
Description
•