Closed Bug 75371 Opened 24 years ago Closed 23 years ago

UI prefs to control pop-up (popup) windows and other Javascript annoyances

Categories

(SeaMonkey :: Preferences, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: Peter, Assigned: doronr)

References

(Blocks 3 open bugs)

Details

Attachments

(7 files, 11 obsolete files)

(deleted), text/plain
Details
(deleted), image/jpeg
Details
(deleted), image/jpeg
Details
(deleted), image/gif
Details
(deleted), image/gif
Details
(deleted), image/png
Details
(deleted), patch
timeless
: review+
bugzilla
: superreview+
Details | Diff | Splinter Review
UI prefs to handle pop-up windows. +--- Advanced Preferences ----------------------------------+ | | | [*] Enable JavaScript | | | | Some web sites use javascrips to manipulate a browser's | | behavior. Use the settings below to adjust which | | maipulations you want to allow, forbid or be asked | | each time. | | | | Action Allowed Forbidden Ask me | | | | Open new window(s) (pop-up) <x>* < > < > | | Load extra images <x>* < > < > | | Change status bar ("ticker") <x>* < > < > | | Detect window close <x>* < > < > | | | | * = default setting | +-----------------------------------------------------------+ +--- Ask Me Dialogue -------------------------+ | This site is trying to open another window | | with the following url: | | www.ruthelesspornsite.com | | | | Do you want to <open> the new window, | | or to <Don't Open> the window. | | | | [ Open ] [ Don't Open ] | | | +---------------------------------------------+ Since most users (including myself) might not know what some of the options do (but are sure to have encountered them), it would be good to have a brief description of each somewhere (tooltip when hovering over an Action?).
Summary: UI prefs to handle pop-up windows. → UI prefs to handle pop-up windows (and other javascripts)
or even better: +--- Advanced Preferences --------------------------------------------+ | | | [*] Enable JavaScript | | | | Some web sites use javascrips to manipulate a browser's | | behavior. Use the settings below to adjust which | | maipulations you want to allow, forbid or be asked | | each time. | | | | Action Allowed Forbidden Ask me | | | | Open new window(s) (general) <x>* < > < > | | Open new window(s) (on leaving a site) <x>* < > < > | | Load extra images <x>* < > < > | | Change status bar ("ticker") <x>* < > < > | | Detect window close <x>* < > < > | | | | * = default setting | +---------------------------------------------------------------------+
I'd prefer a more cookie-like requester on "Ask me": +--- Ask Me Dialogue -------------------------+ | This site is trying to open another window | | with the following url: | | www.ruthelesspornsite.com | | | | Do you want to <open> the new window, | | or to <Don't Open> the window. | | | | [ Open ] [ Don't Open ] | | | | [ ] Remember this decision | | | +---------------------------------------------+ The "Remember" option should be per site, just like it is with cookies.
-> uid
Assignee: asa → mpt
Component: Browser-General → User Interface Design
QA Contact: doronr → zach
mpt already has a spec for this panel in a bug somewhere.
Whiteboard: DUPEME
I noticed Peter Lairo added "Open new window(s) (on leaving a site)". This is already discussed in bug 33448 that I hope makes it illegal for this to happen under any circumstances. Personally I'd much rather see this on a per site basis instead of globally. Or maybe have some sort of JavaScript permission zones so you can specify what sites can use all features.
Found the original bug. That has a UI that handles zones and the like proposed. If you decide to move your UI proposal to that bug, please follow mpt's lead and use the attachment feature. *** This bug has been marked as a duplicate of 38966 ***
Status: UNCONFIRMED → RESOLVED
Closed: 24 years ago
Resolution: --- → DUPLICATE
Looks rifht to me.
Status: RESOLVED → VERIFIED
How many features should that one bug handle. The other bug is more of a meta bug. It has already become too diluted. Let's keep bugs specific and narrowly defined. That is the only way they will be fixed. Reopening.
Status: VERIFIED → UNCONFIRMED
Resolution: DUPLICATE → ---
*** Bug 80336 has been marked as a duplicate of this bug. ***
Confirming. -> Preferences as well.
Assignee: mpt → mcafee
Blocks: BlockJS
Status: UNCONFIRMED → NEW
Component: User Interface Design → Preferences
Ever confirmed: true
QA Contact: zach → sairuh
Whiteboard: DUPEME
nav pretriage: moving to Future.
Target Milestone: --- → Future
over to mpt to dup, give back to me if you want implementation help.
Assignee: mcafee → mpt
*** Bug 86049 has been marked as a duplicate of this bug. ***
*** Bug 87484 has been marked as a duplicate of this bug. ***
*** Bug 90940 has been marked as a duplicate of this bug. ***
Gerv!
Assignee: mpt → gervase.markham
OK, so this bug needs several things before it can be worked on. I assume that it's a request for a new prefs panel under "Privacy and Security". So it needs a title. And probably some UI love from mpt. We also need to know exactly which capabilities prefs will be included, and for someone to tell me the pref names. My currently limited understanding about how this works implies that to do "Ask Me" we need to implement a third option, other than noAccess and allAccess, for the capabilities prefs, which will do a generic asking thing. This may exist already, but I doubt it. In the mean time, we can have "Yes" and "No" without any trouble; we'll start off with checkboxes. Also, can someone please point me to a spec for the Zones work? That doesn't actually look all that complicated, and it would be really good to get this stuff in under that umbrella. Gerv
Gerv, Look at http://www.mozilla.org/projects/security/components/configPolicy.html. That explains the prefs and the 'zones' concept. Do we really need an 'ask me' option? I think we're better off without. Asking the user with a dialog if it's OK to open a dialog seems wrong somehow...
> My currently limited understanding about how this works implies that > to do "Ask Me" we need to implement a third option, other than > noAccess and allAccess, for the capabilities prefs, which will do a > generic asking thing. There also needs to be an "Allow pop-ups from this site." and "Reject pop-ups from this site." - ala cookies. I, for one, only want pop-ups from a few specific sites, so I don't want noAccess but I also don't want to have to be asked every time I visit a site.
The nice thing about the "ask"c dialogue is that it can contain a *Remember this decision* button :) That is the easiest way to populate the allow/disallow prefs lists; and you are only bothered once per site.
I don't like the idea of having a dialog for window.open, because a. Using a modal dialog to ask whether a web page can open a window seems a little silly, as Mitch pointed out. b. You don't know whether a pop-up is an advertisement until after it opens. c. I visit many more sites where I want to disallow pop-ups than sites where I want to allow them. If I choose "ask me", I'll have to bonk 'no' every few minutes. I just started a thread on the newsgroup netscape.public.mozilla.security (and also on n.p.m.ui), with the subject "pop-up ads and denial of service attacks". My post includes several alternatives to having a dialog.
I think the settings for opening the new window should be quite exactly like images - alow, allow all, disallow cross-domain (most ad-popups are crosdomain and I have never seen) any popup in different domain that is not an ad.). And the dialgo will have [ ] remember decision for this domain checkbox It might be handy also to disallow focus(), blur() and similar annoying functions (those should be of course without any dialogs)
I agree with Martin on the focus(), blur() matter. Also, the method suggested by Peter Lairo (first and second messages) and improved by Johan Walles (third message) seems right to me. Marcos.
*** Bug 92465 has been marked as a duplicate of this bug. ***
*** Bug 95155 has been marked as a duplicate of this bug. ***
*** Bug 95588 has been marked as a duplicate of this bug. ***
I'm not going to get to this anytime soon. Other UI ideas over in bug 97564. Gerv
Keywords: helpwanted
Summary: UI prefs to handle pop-up windows (and other javascripts) → UI prefs to handle pop-up (popup) windows
*** Bug 97564 has been marked as a duplicate of this bug. ***
I don't understand. You moved the "(and other javascripts)" comment to bug 97564, and then marked that vry bug as a dupe of this one. That should nullify the removal of "(and other javascripts)" from the subject. I suggest to readd that line to not loose the cohesiveness of the features. Subject: "UI prefs to handle pop-up (popup) windows (and other javascripts)" Or am I overlooking something (obvious)? If backend work needs to be done to implement the "functionalities", then we could create bugs for that, and have this bug depend on them.
I removed "and other javascripts" because it was bad grammar. Gerv
Summary: UI prefs to handle pop-up (popup) windows → UI prefs to control pop-up (popup) windows and other Javascript annoyances
Assignee: gerv → doronr
Target Milestone: Future → mozilla0.9.5
-> moi, 0.9.5, removing ns*- keywords.
Two other cases that should probably be optionally disabled are: - allow popup windows upon entering a site {y/n} and I don't know if this is possible but: - allow popup windows based on timer events {y/n}
"Timer events" is much too technical. Remembering whether the timer was registered by a handler which may not open windows (or rather its URL, so that the "ask" feature works) is much better, IMHO. Speaking of JS filters, the iCab browser supports the following categories: - run ANY script - check referer: header - write to status line - read cookies - write cookies - read history - open windows (no onLoad/onUnload check here) - change window attributes (i.e., hide title bar/button bar/resize/close) - change window location/size and I suspect many more could be added. IMHO that much would be overkill, and I'd rather have a generic solution where I could just add a "replacement" JS function for the original thing which can do additional checks, if desired, and then call the original function if desired. That is much more generic, and opens the way for people to easily submit a lot of specialized filters. In fact, these should be able to override other functions. Like, for instace, the one which loads images... it should be obvious why.
There should be a persite control like for cookies/images too, with the possibility to have a pop-up: Site foo.com want to use the following javascript function: .... Allow it? Yes No [X] Remember this decision
mpt: since popups are really going to be loved, perhaps a seperate checkbox for them, to be more visible? Also, "open windows by themselves" does not really cover the pref
mpt: Is there any chance you could come up with wording for the two new-window checkboxes that won't confuse people who *do* know javascript? I'm worried that "Open web page links in new windows... when specified by the page" sounds like it's referring to target=_blank, not javascript, especially when that radio group is outside of the "Enable javascript" box. mozilla@serialhomicide.com: window.open in an onload event would be covered by "allow scripts to... open windows by themselves", as would window.open in an onmouseover event. With that box unchecked, web pages would only be able to make a new window open when you click on a link. (If you also selected "open web page links in new windows... only when I choose 'open in new window'", web pages would never be able to open new windows for you.) smurf@noris.de: Remembering where a timer event was registered doesn't make much sense to me. I want to allow web pages to open new windows only when I click within the window, not "when I click within the window or any time after that".
I've run across a few web pages which open new windows delayed, mostly because they want to show the user some indocation or result of processing before popping up the new window. I once planned to do that for our own customer service system. It's still in response to a user's click as opposed to an automatic handler, and that's what counts, IMHO. Still, timer events triggered by onLoad are suspicious, as are window-open events based on onMouseOver (which is one not-at-all-silly example which is sure to catch on when disabling onLoad window-open becomes more widespread).
> - allow popup windows upon entering a site {y/n} As Jesse said, that's part of `Open new windows by themselves'. > - allow popup windows based on timer events {y/n} So is that. I.e., unchecking that checkbox should prevent any new windows *except* those caused by `Open in New Window', target="_blank", onclick=, or href="javascript:...". > Speaking of JS filters, the iCab browser supports the following categories: Yes, I had one eye on iCab while designing this. The checkboxes in my spec are a superset of those in iCab (with the exception of `read history', which we should never allow anyway). > There should be a persite control like for cookies/images too That's a different bug. > mpt: since popups are really going to be loved, perhaps a seperate checkbox > for them, to be more visible? I think putting it at the top of the list is quite enough; separating it from the others would be more confusing than useful. > Also, "open windows by themselves" does not really cover the pref If it doesn't, that's a bug (see below). > mpt: Is there any chance you could come up with wording for the two > new-window checkboxes that won't confuse people who *do* know javascript? > I'm worried that "Open web page links in new windows... when specified by the > page" sounds like it's referring to target=_blank, not javascript, It refers to both. > especially when that radio group is outside of the "Enable javascript" box. It's outside the Javascript groupbox because it doesn't apply just to scripts.
*** Bug 101049 has been marked as a duplicate of this bug. ***
*** Bug 101963 has been marked as a duplicate of this bug. ***
The idea is simple. Instead of allowing the browser to either allow or not allow pop-up, have it so every you enter a page that has pop-ups that it would query the pop-ups in the sidebar and you could manually allow each pop-up you want. You could even have it so it only allows a single window pop-up at a time. In the side bar it would include information such as Window size Name of pop-up Location coming from And most importantly see if there is a trigger to load more pop-ups The layout should be something like a directory structure. Starting with the page you are viewing. And then listing the pop-ups like sub directories. Not all pop-ups are bad, just 90% of them.
site specific blocking will be a rfe that will be filed after this goes in
this depended on 101150, which got fixed (big thanks to bz). Will attach first take on this today hopefully
Depends on: 101150
from all.js: pref("capability.policy.default.Window.focus", "allAccess"); that is the only pref in all.js that would be affected with mpt's spec.
Attached patch betaish patch (obsolete) (deleted) — Splinter Review
http://www.nexgenmedia.net/mozilla/pref.png What I currently have, works but needs some refinement.
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.5 → mozilla0.9.6
I really think you should use < > Never, < > Ask me, < > Always for each item instead of yes/no. "Ask me" should say: This web site want to use the following Javascript functions: window.open() window.resize() Do you want to allow it ? Yes No [X] Remember my decision Like for pictures/cookies. En item: "open windows in onload() and onunload() events" should be added too.
Doron Rosenberg wrote in 2001-10-04 12:43 > http://www.nexgenmedia.net/mozilla/pref.png > > What I currently have, works but needs some refinement. Yes, it needs. A few suggestions: "Allow scripts to:" "Open windows by themselves" instead of "Windows to open by themselves", "Make windows flip over or under other windows" instead of "flip over or under other pages", and "Swap images (image rollovers)" instead of "Image Swapping". Marcos.
I think you should heed Marcos' comments. I think this belongs in the Privacy & Security category rather than in a category by itself. Furthermore I think we should have Yes|No|Ask options available just as with cookies and images. Or maybe even something like a positive list of sites that are allowed to (for example) open popups while the rest are blocked. BTW you misspelled "themselves".
- spelling mistakes and wording - needs work, I know - per-site setting: doable, but not in spec :) (add a "view script permissions" button like the image permission pref tab). However, that is a different bug.
> Furthermore I think we should have Yes|No|Ask options available just as with > cookies and images. Actually, that will require some back end work as well (there is really no provision for disallowing a particular function on a particular site on a _one_time_ basis). Doron's implemented a UI for what the back end supports. Note the summary of this bug. If you want more options, lobby for improvements to the back end. And do it in a different bug, please. :)
> If you want more options, lobby for improvements to the back end. The best way to do this is to create a new bug for the backend to add the ability to grant/deny/ask on a per site basis and *make this bug dependent on the new backend bug*. How would the prefs settings then be treated? as global defaults? Do the per site settings override the global defaults (i think yes)? (e.g. if *open new window* is ALLOWED in prefs but FORBIDDEN for site xxx.) For now, we should implement the YES/NO/ASK dialog for the global settings (Doron?) since that functionality is hopefully already available. The danger, however, is that the decision would affect that setting for ALL sites!!! Hopefully, a per-site backend can quickly be hacked from similar backends with the same functionality (e.g., cookies?). In the mean time, the default settings should NOT be "ASK", and if someone does change it to ASK, the dialogue (when a site tries to activate that javascript) should have text that makes it clear that changing the setting will affect *all sites* that call this function.
The "ask me" is currently NOT possible. the per-site limiting is possible via security capabilities, but that is another bug. for the "ask me" feature, perhaps you can extend the security capabilities to allow this. But again, different bug.
As an aside, per-site would be nice. Have a great big "DENY" default policy, then allow some sites (like PA) to have access to the JS so that their silly auto-submit archive selector works (for example). There are also /some/ sites which create new windows on click operations (such as webcams, art previews, so on) which would be nice te be able to turn on. It'd also be nice if some space was dedicated to a toolbar somewhere that let you manipulate the tristate of "default, allow, disallow" quickly and easily, rather than having to dig into the prefs dialog each time.
Attached image Popup Manager In The Sidebar Idea (deleted) —
Filing new bugs for "Ask Me" front end and back end Back end: bug 103405 UI: bug 103406 Maybe that will relieve some of the pressure in this bug.
Popup Manager In The Sidebar Idea Key I tried to use items that are already in the browser to represent the Popup Manager. The Folders represent open windows, that YOU have open manually. The Green lights say it is OK to open the window automatically. The Red Flags say NEVER open the Popup automatically. If the fields are blank, user intervention is required, but by default the popup will remain closed. If the person likes they can manually open a popup by clicking on it in the Popup Manager Site area. If the pop is open this will bring that window to the foreground. You can kill all open popup by selecting the desired windows in the list and pressing the delete key. Underneath the list is additional information about the site. Additional information will give stats on the popup, size of window, location of window, etc? just fun stuff. Open will open the window, and Don?t open will not open the window. The check box I have also included the option to block ALL popup coming from a selected site. If you have any other questions feel free to ask. Also I am moving the developemet of the UI to Bug 103406 http://bugzilla.mozilla.org/show_bug.cgi?id=103406 this is just a heads up for all of you who are intrested in helping.
Attached patch better patch (obsolete) (deleted) — Splinter Review
this patch is much better. Instead of setting |allAccess|, we simply remove the pref via clearUserPref(). http://nexgenmedia.net/mozilla/pref.png shows the new look What is missing are default prefs for the checkbox values.
Wouldn't it be better to split "open windows by themselves" into: 1. open new window(s) on loading a page 2. open new window(s) on leaving a page3. open new window(s) for all other cases (e.g., ???) Reason: new window(s) on-load is sometimes legitimate (e.g. Netscape homepage - OK, debatable), but on-leave is almost NEVER legitimate (e.g. ruthelesspornsite popups). I hope this diferentiation exists in the backend ;) PS. You shouldn't have a checkbox ("--[x] Enable Javascript--") in the titlebox of the pref. How about this: +- Javascript Preferences ------------------------+ | | | [x] Enable JavaScript | | | PPS. Since these are global settings and the user will likely need different settings for different visited sites, I really hope someone comes up with a sidebar tab for these prefs to quickly switch settings while browsing ;)
Doron, I'm glad to see you're open to sugestions :-) What about one more, replacing "Allow scripts to do the following:" to Allow scripts to:", because each item would be completing the sentence, so: "Allow scripts to:" "Open windows by themselves" "Allow scripts to:" "Move or resize existing windows" and etc. Agreed with Peter Lairo (2001-10-07 09:10), "PS. You shouldn't have a checkbox [x] Enable Javascript in the titlebox of the pref..." Marcos.
> Wouldn't it be better to split "open windows by themselves" > I hope this diferentiation exists in the backend Nope. It does not.
> PS. You shouldn't have a checkbox ("--[x] Enable Javascript--") in the titlebox > of the pref. Please see mpt's spec for the UI, which _does_ have such a checkbox. It makes sense to me, since unchecking that makes all the prefs in that box moot (and disabled).
I have one more patch, which does some minor prefs reordering. Still not sure where the scripts panel belongs.
Boris Zbarsky 2001-10-08 13:56 ------- > > PS. You shouldn't have a checkbox ("--[x] Enable Javascript--") in the titlebox > > of the pref. > Please see mpt's spec for the UI, which _does_ have such a checkbox. It makes sense to me, since unchecking that makes all the prefs in that box moot (and disabled). I think he didn't mean that we shouldn't have the "enable javascript" option, he was just questioning it's position, where should it go. Note the *"in the titlebox of the pref"*. Marcos.
--- After re-reading the comments Well, anyway, the way it's done in http://nexgenmedia.net/mozilla/pref.png seems fine to me. Marcos.
I know what Peter meant. The UI spec has the checkbox in the heading. That placement makes sense since that checkbox toggles everything in the box to be disabled/enabled.
I think that is incorrect UI. Maybe some UI "specialist" can provide input?
Attached patch even better, perhaps final patch (obsolete) (deleted) — Splinter Review
http://nexgenmedia.net/mozilla/pref.png patch now has defaults in all.js, removes the "enable javascript" option from the advanced panel. any code comments or perhaps r/sr? cc: ben goodger, who is the prefs master
You should put two open() options: [ ] Allow scripts to open windows in onLoad/onUnLoad events [ ] Allow scripts to open windows in reponse to a use clic The back-end is here, it's the 'dom.disable_open_during_load' option.
Comment on attachment 54435 [details] [diff] [review] even better, perhaps final patch >+ <checkbox id="javascriptEnabled" label="&enbJsCheck.label;" accesskey="&enbJsCheck.accesskey;" Let's not have these cryptic entity names? :) How hard is it to spell "enable"? I have to ask. Why all the content.scripts preferences? They seem to be used only to set the state of your checkboxes... the capability.policy prefs do all the work. It seems like we have a recipe for the two getting out of sync here, no? >+ <description id="allowScriptsDescription" value="&allowScriptsDescriptions.label;"/> Decide on whether "Description" is plural or not and stick with that. :) >+ <checkbox id="allowWindowOpen" label="&allowWindowOpen.label;" oncommand="doAllowWindowOpen(this);"/> What's the point of this oncommand? Why not just use the pref=, prefstring=, preftype= attributes here? That would also mean not having to set it in Init() >+var pref = Components.classes["@mozilla.org/preferences;1"].getService(Components.interfaces.nsIPref); should that not be a "const pref" ? >+<!ENTITY scriptsNwindow.label "Scripts &amp; Windows"> again, let's avoid cryptic entity names. Saving 2 chars is not worth it... -------------------- Quick reply to Gael. The "Open windows by themselves" checkbox sets dom.disable_open_during_load
Attachment #54435 - Flags: needs-work+
With Mozilla now supporting tabs, there should be --- Open popups to ------------- | (X) new windows | | ( ) new tabs | -------------------------------- Same applies to other applications displaying webpages.. --- Open documents from other applications to ------- | (X) new windows | | ( ) new tabs | | ( ) current window | ----------------------------------------------------- [X] set focus to opened document Actually current window is quite useless option, so there might be no need for it.
Reply to Boris: ok, but you still need two options: one for automatic pop-ups and the other for user-triggered pop-ups. Reply to Lasse: there is an option in recent nightlies in the "Browser => Tabbed browsing" section to "Open tabs instead of windows for... Windows opened by the page"
new patch is at http://www.nexgenmedia.net/mozilla/75371.txt, don't want to cause too much new spam
MPT wants to get rid of the advanced prefs panel eventually so finding another place for it makes sense. Someone suggested under "Privacy and Security" which is where the cookies and images prefs were moved to. Also the "Enable JavaScript for Mail and News" option should be moved to this prefs panel to keep all the JavaScript prefs in the same place. It would look strange to someone looking in the 'Advanced' panel for disabling JavaScript and only seen the mail and news option and didn't bother to look further because it's reasonable to assume all the JavaScript prefs will be together.
Gael: `Allow scripts to open windows in onLoad/onUnLoad events' is exactly the same as the `Open new windows by themselves' which Doron has implemented, except that Doron's text can be understood by mere mortals. A mere-mortal equivalent to `Allow scripts to open windows in reponse to a use clic' is in my spec (`Open Web page links in new windows:...'), but Doron hasn't implemented that because the back end is *not* there. Boris: I can't comment on most of the code issues, but the reason for the extra content.scripts.* prefs is that the content policy prefs are far too granular to be useful individually. For example, humans should not have to uncheck eleven different checkboxes (!) just to prevent windows from being moved or resized by themselves; they should only have to uncheck one. It would be cool if mstoltz or someone could amalgamate some of the unnecessarily separate capability.policy.* prefs so that the extra content.scripts.* prefs weren't needed. David: Except for the two checkboxes to do with cookies, these prefs have nothing to do with privacy or security, so putting them in the Privacy & Security category branch would be absurd. It is true that the Images prefs are in that category branch, but we have at least one bug open on the absurdity of that too. The placement in the `Advanced' branch is temporary, because my prefs redesign has no category branches at all. :-)
Perhaps "Advanced behaviour" or "______ Behaviour" under Navigator? I'd also like to see image-style per-site allowances for different ACLs of JS, or a toggle button on the <- -> ^ X bar so that I can turn things on for the proper sites. (Maybe a nice quick-toggle bar for Java/JS props/images/etc.. later on, add DnD so you can make regexps of images to not load, for the IJB features bug 91783)
> The placement in the `Advanced' branch is temporary, because my prefs redesign > has no category branches at all. :-) Cool. Can we see it? Just for now, can't we put the 'Enable JavaScript for Navigator' back where it was (Advanced panel) and add an 'Advanced...' button next to it to launch a window full of Doron's wonderful prefs? It would be inline with the advanced Autocomplete tinkering options but I fear someone will protest that it hides the options away too deep.
Whiteboard: [Aufbau-P1]
Whiteboard: [Aufbau-P1]
> Perhaps "Advanced behaviour" or "______ Behaviour" under Navigator? No, because it doesn't apply just to Navigator. > I'd also like to see image-style per-site allowances for different ACLs of JS No, because as has been noted five times already, that is outside the scope of this bug. If you're not going to read the bug before commenting, please don't comment at all. > Maybe a nice quick-toggle bar for Java/JS props/images/etc No, that's also clearly not part of this bug. It's bug 38521. > Just for now, can't we put the 'Enable JavaScript for Navigator' back where > it was (Advanced panel) and add an 'Advanced...' button next to it to launch > a window full of Doron's wonderful prefs? No. Given the freqency and annoyance of popup windows during the average surfing session, these prefs should not be considered `Advanced' (as I said, their placement in the `Advanced' category is temporary). And having an `Advanced...' button inside a category which was itself called `Advanced' would be ... disturbing.
Keywords: uipatch, review
*** Bug 107716 has been marked as a duplicate of this bug. ***
due to my being gone for almost 2 weeks and the fact that I rushed the last patch pushing to next milestone.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
After talking with Doron on irc i have to quibble with the naming on the allowWindowOpen checkbox. I feel not only is it confusing it is misleading to the intention of the preference. The preference is supposed to (correct me if i am missing/wrong here) disallow the window.open() command while the page is being loaded (or on a timer). While still allowing the use of window.open() on the page itself. Thus the title "Open windows by themselves" while generally correct could be worded a lot better. I suggest something like "Open New Windows during Load" or something to that effect. Thoughts?
The wording should be something that "regular" people can understand. (Has this not already been discussed?) As such, "Open windows by themselves," while perhaps not completely accurate, is the most "user friendly" and most accurate without using syntax that would simply confuse most people.
my $0.02: "Open new windows automatically" Seems clearer without getting technical. "by themselves" is strange.
Actually, I take that back. I can see how automatically can still mean in response to a mouse click if the user just didn't ask for it to be in a new window. Maybe "Windows open by themselves"?
What about: Open new windows and pop-ups while a page is loading It's a bit long but it does mention pop-ups which is what most users checking this pref will want to get rid of.
What about [ ] Open windows while entering/leaving a site [ ] Allow opening of windows (popups) while entering/leaving a site [ ] Allow windows to open while entering/leaving a site [ ] Suppress opening of windows while entering/leaving a site (Block popups/popoffs) ? > // More important, disable JS windows popping up a new window on load > // (as lots of porn and spam sites do): > user_pref("dom.disable_open_during_load", true); <http://www.mozilla.org/unix/customizing.html>
Attached patch cleaner patch (obsolete) (deleted) — Splinter Review
some comments: -if a capability used by one of the checkboxes is "sameOrigin", the checkbox will become unchecked, since we are not allowing all circumstances, and the fact that i f someone is setting capabilities, he doesn't need the panel -since everyone keeps asking this: dom.disable_open_during_load can't be directly used for the checkbox, since I need a negated value
Attachment #52075 - Attachment is obsolete: true
Attachment #52442 - Attachment is obsolete: true
Attachment #54435 - Attachment is obsolete: true
Comment on attachment 58103 [details] [diff] [review] cleaner patch >+ document.getElementById('allowWindowOpen').setAttribute("checked",!pref.GetBoolPref("dom.disable_open_during_load")); This should be in a try {} catch (e) { /* set some default */ } You have a bunch of stuff way over 80 chars long (just like that line). Please line-break them in a reasonable way. Why are you using an Init() method and calling it by hand? initPanel() should call the panel's Startup() method automatically, so just make use of that Also, you should only save the prefs when OK is hit. Register a function to listen to the OK button using parent.hPrefWindow.registerOKCallbackFunc (see what the fonts panel does).
Attachment #58103 - Flags: needs-work+
> if a capability used by one of the checkboxes is "sameOrigin", the checkbox > will become unchecked, since we are not allowing all circumstances, and the > fact that if someone is setting capabilities, he doesn't need the panel sameOrigin, not allAccess, is the default for DOM properties that don't have defaults set in all.js. Window.status and HTMLDocument.cookie.get will almost never be allAccess. Only a few properties, such as Window.focus, are allAccess by default. I think your algoritm will cause many checkboxes to begin unchecked in a fresh profile.
Comment on attachment 58103 [details] [diff] [review] cleaner patch There's a typo in the license header of pref-scripts.js. One more typo and it could read Doron Rose Nerd.
Jesse, Bz: thanks for the info/tips Fabian: bah
Blocks: 111542
Press F12 in OP6 and this dialog comes up. Wording is simple: [ ] Accept pop-up windows [ ] Refuse pop-up windows [ ] Open pop-up windows in background
Attached patch new patch taking in account feedback (obsolete) (deleted) — Splinter Review
New patch reflecting feedback (all I hope, unless I forgot something).
Attachment #58103 - Attachment is obsolete: true
Please move doAllowWindowOpen() to be with other similar functions -- it's lonely on it's own. > + if(prefValue == "allAccess" || "sameOrigin"){ Um.... no. :) you need "prefValue == " after the "||" > + catch(e) { //if no pref set, check box, as it is equivalent with > "AllAccess" Change that comment to "SameOrigin" > +<!ENTITY window.title "Scripts &amp; Windows"> If this is not used and can be removed (whith it seems to be) please remove it. > +<!--LOCALIZATION NOTE (enbJsCheck.label): 'JavaScript' should never be > translated --> > +<!ENTITY enableJsCheck.label "Enable JavaScript"> Make the comment match the actual entity name, please. Fix these minor nits and r=bzbarsky. Please get Jesse's stamp of approval as well.
Attached patch way better patch (obsolete) (deleted) — Splinter Review
The major change is, I can suddenly access the xul directly in doOnOk. Must have been a bad build when I last tried. Also cleaned up the xul a bit, removing useless attributes from <page>
Attachment #59934 - Attachment is obsolete: true
> + allowWindowMoveResizeCheck = true Add a semicolon Also, I would name all these vars "*Changed" not "*Check" Finally, I would be tempted to do something more like: allowWindowMoveResizeChanged = !allowWindowMoveResizeChanged; but you and Jesse should decide which of those is a better idea. Again, if Jesse is happy this has r=bzbarsky
Attached patch yet another sexy patch (obsolete) (deleted) — Splinter Review
cleaned up a bit, implemented the != suggestion.
Attachment #60084 - Attachment is obsolete: true
Comment on attachment 60695 [details] [diff] [review] yet another sexy patch r=bzbarsky
Attachment #60695 - Flags: review+
cc: alecf in case he has time for a sr=. I've been running/using this for some time now.
Before I will superreview this I want to see an updated spec and discuss this with some of the other apps people.
someone want to attach a screenshot of the latest patch in action. Thanks.
Attached image Screenshot of latest implementation (deleted) —
Attached image Screenshot of latest implementation (deleted) —
Could you also add a screenshot showing the new sub-category and the Advanced panel with "Enable JavaScript for Navigator" removed?
The patch appears to be corrupted -- some of the new files appear twice (?). Two questions: 1. How does this interact with the JS pref for Mail and News? At the moment the two prefs are next to each other, as far as I can tell this patch will split them apart. Should the Mail and News pref be moved to one of the Mail and News panels, or should it be moved to the new JS panel? 2. If the current state of the prefs don't exactly match the checkboxes (e.g. resizeTo is disabled but moveTo is enabled) then the user won't have feedback saying that if he toggles the checkbox twice it _will_ change things. I would recommend initially setting the checkboxes to the "indeterminate" state in those cases. As in: +--- [X] Enable JavaScript ------------------------+ | Allow scripts to do the following: | | +----------------------------------------------+ | | | [X] Open windows by themselves | | | | [#] Move or resize existing windows | | | | [X] Make windows flip over or under other... | | | | [X] Change status bar text | | \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/ kerz: When you made your screenshot did you have JS disabled?
Yes, soemthing is wrong with the patch, I must have diffed 2 files twice. Something is wrong in that screenshot, if js is disabled, the checkboxes should become disabled (their labels not, seems to be a general mozilla bug). Hixie - I tried "indeterminate" before and it seemed to not work. Also, if one clicks on the indeterminate (only happens for the window resize checkbox), nothing should be changed, right? Wondering if that would confuse newbies (though one can assume newbies won't get it if they don't change their prefs.js) Jag - http://nexgenmedia.net/mozilla/pref.png is a bit old, but nothing has changed regarding the structure Since I have an exam tomorrow and thursday, I'll only be able to take some new shots later today.
> user won't have feedback saying that if he toggles the checkbox twice it _will_ > change things This is only true if the user toggles the checkbox, then clicks ok, then reopens preferences, then toggles the checkbox, then clicks ok. This sequence of actions, combined with manual editing of prefs.js to put in security policies (why not user.js??). Does "indeterminate" essentially disable the checkbox?
> Does "indeterminate" essentially disable the checkbox? No, it is enabled, but at a state between true and false. It covers two options, one option is enabled, one disabled.
The screenshot was just to show how it looked, not how it acted. I didn't have a full tree, so i just pulled the xul out and took a screenshot of it, without any JS working.
The last patch seems to be broken, the diff is against /dev/null
New files (that were cvs added) are diffed against /dev/null. The diff is fine....
okay if that is the case then... But I can't seem to find the prefs after applying the patch here...
http://www.nexgenmedia.net/mozilla/pref-1.png - advanced panel http://www.nexgenmedia.net/mozilla/pref-2.png - new "scripts and windows" panel I'm aware the latest patch has 2 files diffed twice, wondering how that happened. I'll post a correct one
Attached patch no duplicate diffs (obsolete) (deleted) — Splinter Review
correct patch without duplicate diffs
Attachment #60695 - Attachment is obsolete: true
I don't recommend this because this is a frequently accessed checkbox. A little change like this will momentarily disorient technical support people all over the world. I also don't agree with surfacing control of so many javascript behaviors. I think pop-ups are an annoyance but that's an issue i personally feel the web will resolve by itself in time. On the flip-side, i think some of these controls might be great for a small number of our users, and if we want to offer these options to the user, it shouldn't be at the cost of more pref clutter. They should be buried, and some level of warning should be offered to indicate how it may adversely affect the web browsing experience. Since this is not an item on the PRD, could we perhaps delay this feature until we can "do it right the first time"?
Depends on: 114519
Mozilla has a prd?
Momentarily disorient tech support people? No more so than our dropping of <layer> or S/MIME support, I'm sure, and the world goes on. I get about 15 requests a day through email and IRC for information on how to set the popup preferences, and after they find out (by digging through old release notes, natch) they always ask: "why isn't there UI for this?". I don't think we have a good answer. Our users want this visible in the prefs UI, and this bug seems to have solidified into a good way of providing it. I share kerz's surprise at the mention of a PRD, because I can't find one anywhere on our site. Let's get this baby in for 0.9.7 and see if we're stormed by pitchfork-wielding tech support people. (There are lots of other technologies out there that suppress popups, including simple disabling of JS, so I really doubt it.)
No longer depends on: 114519
I think Hixie is right and we should move the MailNews JS pref to also be on this panel. Does disabling javascript also disable it for mailnews regardless of the state of the mailnews pref? If so, the global JS checkbox should control the enabled/disabled state of the mailnews one. If not, the mailnews one should be below the fieldset and independent. Doron, this should be a fairly trivial change. Could you make it and post an updated patch? > If the current state of the prefs don't exactly match the checkboxes (e.g. > resizeTo is disabled but moveTo is enabled) I somehow get the feeling that the number of users this applies to (tweaked capability prefs, put them in prefs.js, not user.js, didn't just disable access to _everything_) is vanishingly small. I really don't think this is an issue that's worth worrying about....
Why is it called 'Scripts & Windows' and not just 'JavaScript'?
> I somehow get the feeling that the number of users this [edited > prefs by hand] applies to is vanishingly small. If what this patch is doing would create a _problem_ for those users, that would be worth some consideration even for only a few users, to avoid screwing up anyone more than necessary; however, in this case I don't believe that is an issue; I think it is reasonable to assume that a user advanced enough to edit prefs.js by hand is also sufficiently astute to realise that changing things in the preferences dialogs might change his preferences. Thus, I don't think it is necessary to worry about whether using the prefs UI in certain ways might undo certain hand editing of the stuff in prefs.js -- of *course* a prefs dialog will change prefs: that's what it's *for*. prefs.js does carry a warning that it is a generated file; perhaps that warning could elaborate a bit more on what this means, but I don't think it's necessary to avoid allowing the user to generate it via a UI, just to preserve potential combinations of hand editing. The *purpose* of allowing prefs.js to be edited by hand is to allow the user to have a granularity of control that is not available in the UI. That isn't changing. What is changing is only the specific details of what is and is not possible *without* hand editing. > Why is it called 'Scripts & Windows' and not just > 'JavaScript'? Probably because one or two of the settings also apply to windows opened via target= in an anchor tag. Either title makes reasonable sense, IMO.
I agree with shaver, we need some UI for this, now. Let's get some testing on this stuff for 0.9.7.
cc'ing jatin to see what his thoughts are on the panel wording.
scripts instead of javascript because if the script were python or perl or ... it should still be affected by the pref.
Comment on attachment 61096 [details] [diff] [review] no duplicate diffs Due to blake's checkbox foo, parts of the patch broke. Have this already fixed, will attach soon
Attachment #61096 - Attachment is obsolete: true
Attached patch uses .checked now everywhere (obsolete) (deleted) — Splinter Review
new patch uses .checked in the 2 places it didn't before. http://www.nexgenmedia.net/mozilla/test.html is for anyone who wants to test. As for moving the mailnews javascript checkbox, this panel is the wrong one. It should go into the mailnews preferences
Comment on attachment 61285 [details] [diff] [review] uses .checked now everywhere r=bzbarsky
Attachment #61285 - Flags: review+
Suggested wording changes to screenshot in comment #116: Change "Open windows by themselves" to "Open new windows" Change "Make windows flip over or under windows" to "Open windows over or under other windows" Change "Set cookies" to "Write to cookies" Change "Read cookies" to "Read from cookies"
> Change "Open windows by themselves" > to "Open new windows" I though this pref only blocked windows opened "onload" (and such), but still allowed windows to be opened in response to user clicks? I believe the actual version is more precise... Please don't dumb it down too much...
> "Open new windows" That would be false. The pref does not disallow opening new windows; it merely disallows opening of new windows when no action was taken by the user to trigger the call to window.open(). Opening windows in response to user clicks, onchange events, and the like is not affected > "Open windows over or under other windows" That too would be false. The pref prevents windows from being lowered or raised in the windowmanager's window stack by scripts in the page. This has nothing to do with opening windows. The change to the cookie wording is fine and may make those options clearer for some people...
>Change "Set cookies" >to "Write to cookies" Hmm, this also blocks creating cookies. "Write to cookies" sounds like changing information in the cookie
>> Change "Set cookies" >> to "Write to cookies" > Hmm, this also blocks creating cookies. "Write to cookies" sounds like > changing information in the cookie How about "Set new cookies and update existing ones" or simply "Set and update cookies"?
once people decide what wording, I'll post a final patch. "Set new cookies and update existing ones" is too long. I think jatin doesn't want "set" to be used
How about "Create or update cookies"
> How about "Create or update cookies" Or, "Store or change info in my cookie jar" and "Read stored info from my cookie jar" Just a thought. "Create or update cookies" is probably fine too, if we just want to be straightforward and not get cute. I agree that "Open new windows" and "open windows over and under other windows" would be misleading and that the earlier wording is therefore better. As odd as "by themselves" sounds, it really does succinctly capture the idea of what is being allowed (or not) by this pref. After some thought, I have decided that I like "Open windows by themselves" just about as well as any way it could be worded. (Just a data point; I don't cut any more ice here than the next user.) However, "Make windows flip over or under other windows" does seem needlessly verbose. How about one of these: "Place windows over/under other windows" "Raise and lower windows" "Change which window is in front" The last is an oversimplification, since it doesn't really cover the case where the foremost window stays foremost and others switch in the background. But most users never think about the ordering of any windows other than the one in front, and those who do are probably power users and can probably figure out that if a website can change which is in front, it can probably also change which is next. Are the terms "raise" and "lower" still considered technical? I would consider "focus" technical, but I'm not sure about "raise" and "lower"; they seem somewhat more intuitive... Perhaps "Place windows over/under other windows" is a better compromise; not too technical, but it does cover, I think, the whole breadth of what is being allowed (or not) by this checkbox.
Three more alternatives: (1) "prevent annoying pop-up windows"; (2) "open one window at a time"; and (3) "open one window at a time, and when I close a window, don't let another one open".
How about: "Open windows by themselves on loading and leaving a page" "Flip windows over or under other windows" or "Bring windows forward or send windows to the background" "Create and update cookies"
> "Open windows by themselves on loading and leaving a page" That would also be untrue. The pref blocks calls to window.open() from functions invoked via setTimeout, for example... The fact is, the current wording for that one checkbox is pretty much optimal, imo.
> > "Open windows by themselves on loading and leaving a page" > That would also be untrue. And excessively verbose as well. > The fact is, the current wording for that one checkbox is > pretty much optimal, imo. That was the conclusion I came to. Eventually. My reasoning is something like this: some of the others, I can think of ways to word them that seem better to me. Others seem fine just as they are. That one, I keep thinking there *ought* to be a better way to word it, but I can't think of one. I'm pretty creative with thinking of alternate ways to say things, but I can't come up with another way to say that one that isn't one of three things: misleading, awkwardly lengthy, or excessively technical. The current wording ("open windows by themselves") is neither misleading nor technical, nor is it particularly long, and I have concluded that it is just about as graceful (i.e., as little awkward) as is possible without being either misleading or technical. Yeah, it sounds a little odd, but the suggested improvements are substantially worse in one way or another.
May as well add my opinions of each and every one: "Open windows by themselves" See previous comment. "Move or resize existing windows" This seems as clear as it can be, and I don't hear anyone complaining about it, so it's probably fine as it stands. "Make windows flip over or under other windows" Long. See comment #137. "Change status bar text" Perfect. Clear, concise, accurate. Don't change it. "Change images (image rollovers)" A question: does this apply just to rollovers (i.e., when the pointer rolls over the image it changes) or does it apply to all cases of a script switching an image? Perhaps "Change images (e.g., rollovers)"? "Set cookies" See comment #137. "Read cookies" Someone said make it "Read from cookies", and that seems okay to me. Or "Read information from cookies", perhaps, though that starts to be long. Are there some of these that we can all agree on? Which ones do we still need to discuss? It would be nice to wrap this up soon so it can make 0.9.7 if possible...
Since the header reads: "Allow scrips to do the following:", I would suggest: *Open new windows (e.g., pop-ups)* Alternatives: - Open new windows - Open windows (e.g., pop-ups) - Open windows I think the "by themselves" part sounds silly (are they lonely? Are they physically separated from the other windows on the x/y coordinates of the screen?). The term "new window" is IMO more clear, shorter, and the commonly used terminology for what is happening.
I also don't really like the wording for moving windows over/under other windows (i.e., "flip" - makes it sound like the page will be upside-down). Maybe it would be better to say: - *Move windows over or under other windows (e.g., another form of "pop-ups")* Alternate: - *Move windows over or under other windows* <-- no "(e.g. )"
> Since the header reads: "Allow scrips to do the following:", I would suggest: > > *Open new windows (e.g., pop-ups)* The problem with that is that the pref doesn't prevent new windows being created in response to a mouse click (for example). The best alternatives I can think of are "Automatically open new windows" (bit too vague), "Open new windows spontaneously" (makes it sound like new windows will open for no reason - perhaps not too far from the truth) or "Open new windows without prompting" (doesn't quite encapsulate the fact that new windows can be created in response to something the user does).
"Open windows without user interaction" or "Open windows without user interaction (e.g. popups)" It is a bit long, and might be a bit technical... But I think it encapsulates the function quite well. I think the wording as-is is quite horrible, and it should be changed. But if the current wording goes in, I won't protest though.
>> Since the header reads: "Allow scrips to do the following:", I would >> suggest: *Open new windows (e.g., pop-ups)* > The problem with that is that the pref doesn't prevent new windows being > created in response to a mouse click (for example). It doesn't need to prevent new windows created in response to a mouse click, or address it. We want such windows, if we actually click on a link, do we not? With the header, the description would be "Allow scripts to ... open new windows (e.g., pop-ups)." This seems perfectly clear to me. It's the script opening the window, not a mouse click, that we want prevented. As it says.
>>> Since the header reads: "Allow scrips to do the following:", I would >>> suggest: *Open new windows (e.g., pop-ups)* >> >> The problem with that is that the pref doesn't prevent new windows being >> created in response to a mouse click (for example). > > It doesn't need to prevent new windows created in response to a mouse click, > or address it. We want such windows, if we actually click on a link, do we > not? > With the header, the description would be "Allow scripts to ... open new > windows (e.g., pop-ups)." This seems perfectly clear to me. It's the script > opening the window, not a mouse click, that we want prevented. As it says. But what if you've got a javascript:window.open link? User clicks the link and a new window opens, but it's still a script that's opening the window and the pref's not going to stop that (and you're right, we don't want it to).
> But what if you've got a javascript:window.open link? For the sake of keeping the prefs understandable, I think we should consider it the "mouseclick" that is opening the new window, even if the mouseclick is calling a javascript. These prefs are for javascrips doing things that the user did not initiate (other than browsing just to a web page). I think this will be understood by all.
ok. Whatever you guys do, please do not use the string 'scrips'. typos should not be visible to the user. (and ideally should not be anywhere in the codebase, or bugzilla.) This post was run through nc4's mail spell checker.
> Before I will superreview this I want to see an updated spec Comment 131 thru comment 141 are basically a slow-motion replay of the thoughts that I thunk when I came up with the `Open new windows by themselves' wording in the first place. And I see no difference in accuracy between `Read cookies' and `Read from cookies', so the shortest one wins. However, Jatin does have a good point that `Set cookies' could be clearer without losing too much brevity. Therefore, the updated spec is exactly the same as the original spec in attachment 49497 [details], with the exception that `Set cookies' should be changed to `Create or change cookies'. Doron's code does not implement the spec in its entirety, nor was I expecting it to, and nor do I think it needs to; the implementation is coherent and highly useful in its current form. I would be disappointed (because of the delay), but not surprised, if sr= was not given until the mail/news JS checkbox was moved below the group box (while we remain in the bizarre world where three applications of different genres share a single prefs dialog). Response to half a billion comments: * It's `Scripts & Windows' because (a) scripts being `Javascript' is irrelevant to the user, (b) like Timeless sez, one day they might *not* be just Javascript, (c) a lot of these prefs are to do with windows, and (d) when the spec is fully implemented they'll be even more about windows and not just about scripts. * It's `Open new windows by themselves' because (a) it doesn't apply to user clicks, (b) it doesn't even apply to those user clicks which trigger scripts, (c) it doesn't apply just to onload and onunload, and (d) every other option suggested is unfortunately either too obscure or too verbose.
> I think the "by themselves" part sounds silly Then suggest a way to say it that doesn't sound silly. "without user authorisation" is too technical. "not in response to user initiation" is even worse. I thought of "spontaneously" and almost suggested it, but I concluded that despite not being technical per se, it is nevertheless a word that, unfortunately, a lot of users would not understand. "unprompted" is another possibility, but I fear it would be more confusing than enlightening for many users. "without a mouse click" may not be completely accurate and is long, and then you have alternate pointing devices... I keep coming back to "by themselves" being better than anything else I can suggest. > For the sake of keeping the prefs understandable, I think > we should consider it the "mouseclick" that is opening the > new window, even if the mouseclick is calling a javascript. For the sake of keeping things understandable, we should redefine the term "javascript" to mean "only javascript that happens as a result of page load or unload, not based on user interaction such as a mouse click"? Then you have the whole grey area of rollovers... Ick, ick, ick. No, all javascripts are still scripts, whether they happen based on user interaction or not. You really want to introduce the need for a lengthy explanation about what does and does not constitute a script to which this whole panel applies, in order to remove two words from one of the checkboxes? Please try to think through the implications of your suggestions.
> * It's `Open new windows by themselves' because (a)... This makes it sound like the window *itself* is opening a window, not the javascript. At best, it is redundant: "Allow scripts to ... open windows by themselves" - it just doen't sound right (is the script opening the window, or the window doing it "itself"?). It seems that "Allow scripts to ... open new windows" would be sufficiently clear. RE. your reasons: a) "Allow scripts" doesn't apply to user clicks either - it's the script that's doing stuff. b) To the user there is no difference between <link target=blank> and a script opening a new window. So this situation is irrelevant from the users' viewpoint. c) "Allow scripts to ... open new windows" isn't limited to "load/unload" either. d) "Allow scripts to ... open new windows" is neither "obscure", and it is definetely not "verbose" ;) Therefore, I think that the pref should be either: Allow scripts to ... "open new windows" or "open new windows (e.g., pop-ups)" <-- my favorite, because many users will recognize the term "pop-up" and only then really understand what is being allowed/disallowed.
"Allow scripts to ... open new windows on (un)load" or "load/close" or "open/close" pi
RE Comment #152: I just think that "by themselves" causes more confusion than clarification. Most users will understand what is meant by "Allow scripts ... to open new windows" (Here the *script* is the *initiator*. If it where a mouseclick that *initiated* a script, which then opened a window; it would still be the *mouseclick* that *initiated* the sequence.). <-- sorry about all the *formatting*, I got carried away (by a script in my brain) ;) I doubt anyone will even notice that when they click a link and a new window opens, that it may have been a script that did it. Everybody will know what is meant by this pref, I'm certain of that.
My grandma doesn't know what a script is. She just wants to stop pop-ups. Therefore Mr. Rosenberg, please consider this grandma-friendly dialog: +--- Annoying Behavior --------------------------------------------+ | | | [*] Enable JavaScript | | | | Some web pages use JavaScript to manipulate browser behavior. | | You can decide what behavior you want to allow: | | | | Allow web pages to: | | [ ] Pop-up windows automatically | | [ ] Move or resize windows | | [ ] Change window focus | | [ ] Change status bar text | | [ ] Set cookies | | [ ] Read cookies | | | | [ OK ] [ Cancel ] [ Help ] | +------------------------------------------------------------------+
Nix on "change window focus". Your grandma almost certainly has *no* idea what that means. Substituting "pop-up" for "open" has a certain merit. It's colloquial, but the meaning seems intuitive to me, now that I think about it... "Automatically" I had considered before and rejected, but now I don't remember why I rejected it. Maybe just because it's a six-syllable word, but I don't think it's really all that technical. What does anyone else think about "automatically"?
might I suggest the following wording: `Open unrequested new windows' this excludes those in responce to user action, i.e. a mouseclick
Responses to the last few comments: MPT (comment 151) 'Create or change cookies' is better but I preferred _basic's 'Create and update cookies' (comment 139). First of all, 'or' might sound like the pref is mutually exclusive (scripts can EITHER create cookies OR change cookies) and secondly, 'change' may give some users the impression that the pref will allow scripts to *change* arbitary cookies rather than *update* their own. A related point: if the user has chosen to reject all cookies in the Cookies pref panel, will the cookie checkboxes in the 'Scripts & Windows' pref panel be disabled? RC (comment 156) RC's ASCII art panel has an explanation of what JavaScript is. I think this is needed. For those of you who hate to scroll, RC's wording was: "Some web pages use JavaScript to manipulate browser behavior. You can decide what behavior you want to allow:" I quite like this wording with the following caveats: * Grammar check: I think "what" should be "which" * "You can decide what behavior you want to allow" sounds like the prefs cover all possible JavaScript functions when they do not. * I think it's important to state that functionality may be limited if JavaScript is disabled. Therefore, I propose: "Some web pages use JavaScript to manipulate browser behavior. You can decide which of the following JavaScript functions you want to allow. Unchecking an item may limit the functionality of some pages." Not too sure about "functions" but I can't think of a better word right now.
> "Some web pages use JavaScript to manipulate browser behavior. You can decide > which of the following JavaScript functions you want to allow. Unchecking an > item may limit the functionality of some pages." > Not too sure about "functions" but I can't think of a better word right now. Just leave it out altogether. The shorter the description the better, and I'm sure people will understand: "Some web pages use JavaScript to manipulate browser behaviour. You can decide behaviour you want to allow. Unchecking an item may limit the functionality of some pages." No need to mention JavaScript a 2nd time, or "functions" at all.
Attached patch goatastic patch (obsolete) (deleted) — Splinter Review
http://www.nexgenmedia.net/mozilla/pref-2.png - changed "set cookies" to "create or change cookies" Also, added headertitle="" to <page> as per the latest round of preference changes.
Attachment #61285 - Attachment is obsolete: true
I think RC's "grandma-friendly" dialog has it nearly right on the money, other than: * Go with Jason Bassford's: "Some web pages use JavaScript to manipulate browser behaviour. You can decide which behaviour you want to allow. Unchecking an item may limit the functionality of some pages." descriptive text (added a missing 'which' in there). * Definately change grandma-UNfriendly "Change window focus". Probably to "Raise or lower windows" would best for granny. Hopefully we can all agree on something like this, wrap it up, put a stamp on it, and ship 'er out. I know I as well as granny would love to see this in place.
I agree with the "Open unrequested new windows", but feel we should wrap this up, review and get this perfectly adequate patch in, and if people want to change the wording, we can have a new bug filed for that. We could discuss the wording for years without any resolution :-)
How about "Open new windows without asking." instead of "unrequested" which might be a bit of a mouthful for granny?
How about keeping the dialog box simple, and having a "Explain this" button, that takes the main browser window to a help page with more detail?
I agree with comment 163 (i.e., we could argue the wording for years without full agreement, and we don't want to delay this for years). "Explain This" sounds like an interesting possibility that could be discussed later, when something basic is working, probably in a separate bug.
bz, could you review the latest patch? 0.9.7 branch is soon :)
Comment on attachment 61459 [details] [diff] [review] goatastic patch r=bzbarsky. Played with it, and it looks decent and works well. :)
Attachment #61459 - Flags: review+
Comment on attachment 61459 [details] [diff] [review] goatastic patch Looks good, mpt likes the UI, sr=shaver. I have mild JS nits, but nothing that should keep this out of 0.9.7.
Attachment #61459 - Flags: superreview+
a=brendan on attachment 61459 [details] [diff] [review] for 0.9.7 (but why the crabbed ){ function close-param-list-open-body throughout?). /be
Blocks: 114455
Keywords: mozilla0.9.7+
Apologies for not weighing in on this sooner, but I have (finally) tried the pref panel out, and it looks great. Let's check this in, close this bug, and start discussing what, if any, features to add on the next go-around. We have barely tapped the power of this interface.
A couple things: a) I like that we've surfaced the pref for pop-ups and it's working very nicely, great job for getting this in. b) I filed another bug for polishing this pref up as far as UI, wording, and what we should be surfacing in this pref http://bugzilla.mozilla.org/show_bug.cgi?id=115353 c) Controlling pop-ups are one thing, I don't think we should have all these prefs for other javascript functions "annoyances". (who's making that call on what functions are annoying anyway, outside of pop-ups which does have consensus) How many prefs for javascript do we want to present to the user is the question? I like having just one, javascript on or off. If we're going to have a pref for pop-ups (or windows options), let's have a pref for how windows are opened. And leave it at that, e.g. let's not try and control whether cookies are dropped and read from this preference (that's what the Cookie preferences under Privacy are for).
"Who's making the call on what functions are annoying or not?" The user, of course, when he/she/it sets whatever preferences are available to best reflect his/her/its feelings in that regard. The more preferences that are available, the better the settings can reflect what each user does or doesn't find annoying. And I *would* like to see some more preferences with regard to opening new windows, including those which open due to target attributes as well as by JavaScript. Possible choices for handling target="_blank" or target="UndefinedWindowName" could be: [ ] Open in new window [ ] Open in new tab [ ] Open in current window
Personally, I feel that we should add the UI for every javascript annoyance that we block as a pref and list them in order of what we think is most important. The white box in the screenshot can be made to scroll.
See also bugs I made for further controlling the activity of popup windows: Bug 114993: Max level of popup windows Bug 114994: A sidebar for denied popup windows
> c) Controlling pop-ups are one thing, I don't think we should have > all these prefs for other javascript functions "annoyances". (who's > making that call on what functions are annoying anyway, outside of > pop-ups which does have consensus) They're all defaulted to on anyway; pop-ups is at the top of the list because the largest number of people hate it. Users who don't care about most things don't need to look all the way down the whole list, unless they're curious. Me, the one thing on the list that I deeply care about is disallowing the JS from changing the statusbar text. There were times with NS4 when I would have happily closed a dozen extra windows to be able to consistently see the correct information in the statusbar for a particular page (especially, the target URLs for links). > How many prefs for javascript do we want to present to the > user is the question? I like having just one, javascript > on or off. That *might* work if the Prefs Toolbar were more universally installed and visible, so that it could be toggled when going to certain sites. Still, that would really be a workaround. *This* is the correct solution. Javascript does a number of useful things, and a number of extremely useful sites (research services, particularly) require it. More-or-less legitimately. Most of these sites are well-behaved with it and don't do anything nasty -- but then you have a whole lot of less scrupulous sites out there that don't actually have any real use for JS but make a complete nuissance of it, by doing things that JS should never have been able to do in the first place. Pop-ups are really only a very small part of this. Anything that gives the website gratuitous control over parts of the UI that are not part of the document or the site (such as features the browser provides for the user's convenience) should be able to be disabled by the user. Easily. Without disabling Javascript from functioning in its important role within the site's content, manipulating the document itself, forms, et cetera.
Quick question: I've been assuming that this stuff only applies to JS that comes from the web -- that chrome is exempt from these restrictions. Is that right?
as some might know, the patch got backed out (from branch and trunk) at my request because of a bug (if you change anything in the panel and move to another panel, pressing ok will cause an javascript error). I already have a new version which fixes the issue, and once I've done testing it fully, will attach it.
Attached patch correct patch for 0.9.7 (obsolete) (deleted) — Splinter Review
New patch that does the correct thing (saves checkbox values even after panel switching). I apologise for this oversight of mine. The patch is meant for 0.9.7 should it make it, it does not fix the other issues brought up (moving the mailnews js checkbox and wording)
Attachment #61459 - Attachment is obsolete: true
Attached patch new, timeless-nit-free patch (deleted) — Splinter Review
fixes some nits from timeless
Attachment #61964 - Attachment is obsolete: true
Attachment #62007 - Flags: review+
Comment on attachment 62007 [details] [diff] [review] new, timeless-nit-free patch Instead of using that rv variable, why not just return early? Otherwise, sr=blake, test prefs well.
Attachment #62007 - Flags: superreview+
QA Contact: sairuh → bsharma
a=asa (on behalf of drivers) for checkin to 0.9.7
No longer blocks: 114455
doron: is there any reason why this was checked into the branch, but not into the trunk? many people are interested in this.
"Open windows by themselves" blocks only popups + popoffs (dom.disable_open_during_load). When will the pref for blocking all new windows (e.g. onclick) (capability.policy.default.Window.open) be added?
I'll second Christian's question. I'm now using the 12-27 trunk build and it's still not there. Why has this been checked into the 0.9.7 branch only and NOT the trunk?
I "third" Christian there, but I guess the reason is that the trunk will see a prefs panel with more options compared to the one in the 0.9.7 branch.
But if it's going to take a while to get the "new" prefs panel ready, why not check in the existing one so that those of us who are using trunk builds don't have to do without this feature entirely?
Am I the only one who thinks a server-specific solution would work better for limiting JS, similar to the way the images work? Is there already a bug for this feature/improvement? The images options blocks ads from certain servers. This would block pop-ups from certain servers. It's worth noting that some servers and web applications actually use pop-ups for the Forces of Good. (For example, web-based IM clients.) You could also try blocking certain events, such as OnLoad and OnUnload. Of course, I think that would open a whole new can of worms...
since the first impl landed on the branch, 0.9.8. As for why it never went into the trunk, I guess we were hurrying to get it into the branch and forgot. If anyone wants to check it in, feel free to :)
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Brendan Byrd/SineSwiper, see bug 38966 for a per-server version of this stuff. I mailed timeless and asked him why he didn't check this into the trunk. He said that he wanted some changes to be made before he considered the patch to be of sufficient quality to do so.
sorry i was distracted. the only thing i could find that i wanted to do was to satisfy blake's requirement. so i committed the patch w/ that change plus whitespace fuzzing. if there are problems with what i committed, please file a bug against me. if there are enhancement requests please file a bug against mpt/doron. this bug is finished.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago23 years ago
Resolution: --- → FIXED
Uhm, timeless, that still doesn't explain the question that is burning on all of our minds: Why isn't this patch in the nightly builds? (I'm using 2001-12-28, win98)
PLairo because timeless just checked it in the trunk. Try the next nightly.
Verified on linux
Status: RESOLVED → VERIFIED
verified on win32 (2001-12-30-08)
Looks like this may have bitroted my patch in 116748 which adds .defaultStatus to .status for the "Allow JS to change my status bar" pref. I'll try to redo it soon, unless whoever made these changes wants to fix it up.
Please open a new bug for that. This bug is fixed and verified. the UI is in. If there's something wrong with it... that's a whole different bug.
Would it be possible to add "Hook on mouseevents". I really hate sites that show winow.alerts when you rightclick the site or those trailing cursors.
New bugs, and stop spamming people on this one. This bug is VERIFIED FIXED.
new bug! I must stress that this bug is done with!!!
*** Bug 118441 has been marked as a duplicate of this bug. ***
*** Bug 69290 has been marked as a duplicate of this bug. ***
Jatin, help needs to be updated to reflect the movement of the Enable Javascript prefs. And what are we doing to ensure that 6.x users know where to find the Javascript toggle in the next release, now that we moved it out from under them?
The commericial builds will not likely have the same preferences, according to Todd Pringle. Todd, am I correct? I will make any necessary changes based on how it looks on the commercial side.
Jatin - see http://bugscape.netscape.com/show_bug.cgi?id=11551. Commercial builds will not have this preference so this change will only affect Mozilla builds.
Lisa, as I understand from that bug, only the "unrequested windows" pref will be removed. It doesn't seem sensical to remove the entire panel...?
Jatin, pop-up blocking is a killer feature. Please do not strip it out for commercial.
Hi Blake. I don't know which parts will not be in the Mozilla build (I was only referring Jatin to the bug which mentions the commercial side). I will ask Jatin to contact Netscape Marketing to get the final word. Thanks.
sorry for extra spam... I meant "commercial" build rather than "mozilla" build in my previous comment.
Jesse, The commercial build will always be missing killer features (in particular, any that were added since the last major branch; currently, for example, tabbed browsing (!) and print preview). This is unavoidable, because of the time required to generate the commercial build, quality-test it, and so on. That's a tradeoff you accept when you decide you want the bundled commercial stuff (integrated AIM client, the spell checker, and so on) and the trademarked icons and things. Anyway, requests specific to the commercial build do not really apply here; this is the Mozilla bug database, and applies to the working codebase from which the commercial builds are _derived_. Hence, VERIFIED FIXED even though there's not yet a Netscape release that uses anything from this bug. For the commercial builds Netscape has a separate tracking system I think (though I am not sure how accessible it is to the public; I can't get bugscape.netscape.com to resolve from here or from work).
Blocks: popups
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: