Closed Bug 75371 Opened 23 years ago Closed 23 years ago

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

Categories

(SeaMonkey :: Preferences, enhancement)

Product:
SeaMonkey
Component:
Preferences
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.8

People

(Reporter: Peter, Assigned: doronr)

References

(Blocks 3 open bugs)

Details

Attachments

(7 files, 11 obsolete files)

Spec in glorious wide-screen ASCII art
2.24 KB, text/plain
Details
Popup Manager In The Sidebar Idea
52.81 KB, image/jpeg
Details
Screenshot of Opera6 pop-up preferences
137.26 KB, image/jpeg
Details
Screenshot of latest implementation
3.55 KB, image/gif
Details
Screenshot of latest implementation
3.55 KB, image/gif
Details
indeterminate (triple-state) ckeckbox (Windows)
13.00 KB, image/png
Details
new, timeless-nit-free patch
16.25 KB, 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?).
Keywords: helpwanted, mozilla0.9, nsCatFood
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: 23 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
Keywords: helpwanted, mozilla0.9 → mozilla0.9.1, ui
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
Keywords: nsCatFood → nsbeta1-, nsCatFood-
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
Keywords: helpwanted, nsbeta1-, nsCatFood-
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) — — 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.

    
    

    
  
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)
        — 
          — 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)
        — 
          — 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: ui → patch, 
          
            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)
        — 
          — 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

    
    

    
  


  
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)
        — 
          — 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)
        — 
          — 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.

    
    

    
  
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)
        — 
          — 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)
        — 
          — 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)
        — 
          — 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)
        — 
          — 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

    
    

    
  


  
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: 23 years ago → 23 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

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: