if we do this can we kill the popup in tabs pref? The only real reason to use it, is to make closing popups easier in my experience. Maybe roll that pref into the tabs by default pref.

With popups blocking we could remove it prolly.
I'm not still sure about this myself though, it doesnt feet very 
nicely the just work gnome philosophy ... on the other hand people 
will bug us for it.

Created attachment 16229 [details] [review]
Bleah

So I ported most of galeon code but I discovered I dont really like
they way they do it :/ I think all popups are blocked and then you can
remove sites from the black list in pdm.
I havent tried it ... but it seem like a bad mix of mozilla and
phoenix implementation :/
If someone with cvs head would try out it in galeon would not be bad
though ...
I hope parts of this code can be reused when we decide to impl it with
a better ui.

So the backend is actually a whitelist but the ui is a bit confusing.
We can use it though.
So 1 we need to design a nice ui, Phoenix is a good start I guess but
it's not accessible (click on the statusbar icon) 2 We need to
complete implementation ... I think it's not going to take more than 2
h (xan hours obviously) once we have a design.
Still, wouldnt be a "make popups less annoying" approach better ?

Shortly, ui nazi need to develop an opinion about popups blocking soon ;)

Not sure what the patch was supposed to do.. I can't see any visible
effect. From reading it I thought it would catch events from Gecko
user_pref("dom.disable_open_during_load", true) and show them in the
statusbar, but I can't see that. The pref at least is active, though. :)

There was a bug in the patch, ephy_embed_single_free_permissions()
isn't included but was referred to from
pdm_dialog_site_permissions_free (I just commented that line out to
make it compile).

The patch is not complete. I stopped on it because I dont know if we
wants blocking and I dont have an ui design for it.

I have no good suggestions for ui here, partly cuz i'm not sure what is needed. I guess my first question is what kind of popups are people
trying to block. Is it fair to assume that popups should never be
blocked in you intentionally click on a link? We need to develop use
 cases i guess. I know of a few band sites that popup windows when they
 are loading that they use as their main interfaces (which sucks but i
 think this is somewhat common on the internet). Can we include some
 smart popup protection perhaps? Maybe block any popup associated with
 double click?

Just throwing ideas around. what do safari/camino do?

I dont think we have much design freedom with mozilla api. There is a
white list and all not interactive popups (click) are blocked. We can
add stuff on the white list and we are notified when a popup is
blocked. Anything smarter than that is way future ;)
Ihmo the choice is
1 clone phoenix popup blocking, adding an accessible way to unblock
2 "make popups less annoying" way

That's why I was proposing to ask desktop-devel. It seem to me more
like a choice then a design thing. Personally I feel popup blocking
overrated, but that can be very well just me.

yeah i think popup blocking is a little overated, but on the other hand my web habits keep me from interacting with pages that gratuitously overuse popups. there are some everyday sites that are pretty bad popup offenders (cnn comes to mind) so it is an everyday web user issue. I guess in my ideal world, we would just intelligently block popups from certain known offenders :/ 

I think the Mozilla logic for which popups are blocked is solid: block
access to window.open() during onload, onunload and timers, and permit
them otherwise (basically meaning to allow them if the Javascript is
executed because of a user click). Not much that can be done about it,
and not much that should. The real issue is indeed how to present the
fact that a popup was blocked, and how to disable that in specific
situations.

A suggestion, I don't know if this is completely doable with the
Mozilla api:

1. when a popup window is blocked for the first time, display an alert
explaining "Epiphany detected that a web site {I think this can be
identified from the dom, show the window title} tried to open a popup
window you have not asked for. Generally these popups are advertising
or designed to stay in the background and monitor your surfing.
Because of this, the popup was blocked. If you want this popup to
open, click on the popup blocked icon in the statusbar.". Include a
"do not show this again" checkbox/button in the dialog.

2. display a gently blinking/throbbing icon in the statusbar (wrt
Windows XP, where a minimized/background window will blink in the
taskbar to draw your attention but not completely distract you from
whatever else you're doing). Keep on running that animation until: a)
user clicks on a link in that window/tab, loading a new page and
making the popup irrelevant, b) sufficient time is elapsed for the
popup to age too much (15 seconds?)

3. (the tricky part, which Mozilla doesn't do to my knowledge)
when the user clicks on that icon, bring up a popup menu listing the
last blocked popup(s) (there may be more than one on a particularly
annoying page). If the user selects one, ALLOW THAT WINDOW TO OPEN.
That requires being able to delay the decision of whether the popup is
ok or not, which would also delay the execution any other Javascript
code in the same function. If that's not possible, present whatever
other information still available about the popup, and allow the user
to decide whether similar popups should be permitted in the future.

4. For popups that are already allowed, there should be a way to
reverse that decision and block them in the future. There are several
options, such as a popup menu item etc, but I'd suggest to force popup
windows to always have statusbars where an icon for blocking the popup
later can be presented (having that statusbar there is good for
security anyway, and at least used to be mandatory for JavaScript
windows to distinguish from other windows).

This is how Phoenix does it. One issue is that 2 need to be made
accessible with keyboard in some way.

Oh, and regarding "popup blocking is overrated", cnn.com is not even a
big offender compared to wsj.com and some portals which allow x10
popup/popunder windows. Pornzilla is already targeting adult sites, so
I don't think we have to. :)

Also, in a perfect world there would be no offenders, but in this
world we can't know who the offenders will be ahead of time. That's
why Mozilla popup blocking works the way it does (and it's really a
godsend: even though I don't frequent that bad sites, just one day of
trying to use Epiphany almost made be go back to Phoenix until I
discovered I could turn on dom.disable_open_during_load through
about:config).

Don't underestimate the importance of privacy features: popup blocking
and selective cookie blocking are the two biggest reasons why my
friends use or consider using Mozilla/Phoenix instead of IE. Nothing
else could overcome the obstacle of IE already being there and
familiarity.

Yeah, that's pretty similar to Phoenix, except for a couple of small
touches:

1. this should be on by default (see my comment in
http://bugzilla.gnome.org/show_bug.cgi?id=107081)

2. I don't think Phoenix delays the popup, it just blocks it
immediately (could be wrong, though, but if I am, good, because it
would mean delaying is possible)

3. for the feature to be on by default, it needs to be somewhat more
visible that in Phoenix default theme, hence the blinking/throbbing.

(I gotta learn to call it Firebird, though).

What is over rated is the number of people that would be able to use
the feature. I dont expect many non technical users to want to deal
with a white list.
The feature doesnt require an obtrusive interface, so it wouldnt even
damage such users. I'd just hate if implementing popup blocking would
delay a solution that works (== at least make popups less annoying)
for everyone.
I agree very much with this analysis of the problem:
http://www.mozdev.org/pipermail/epiphany/2002-December/000043.html

I'm really unsure how well the proposed solutions would work though,
and if they are implementable at all.

Finally I dont have a strong opinion on the issue, maybe at some point
I'll develop one ;)

I dont think 1 and 2 are possible (with current api).

That email goes wrong in one important detail (might have been closer
to truth at the time, I don't remember when Mozilla popup blocking
matured to what it is now): the popup blocking heuristic, as it is
now, is at least 95% percent effective in real world usage:

 - popups the user didn't ask for almost very rarely contain anything
important, and almost never disable anything. The only example I can
come up with is the first-time-cnn.com-visitor, who gets a popup to
select their "edition" (US or International), which gets saved as a
cookie. The same thing can be done from the main site as well, and
it's hardly a critical feature. Legitimate uses of onload popups are
very rare. The whole thing is a misfeature from the naive days of
Netscape 2.0, and most web designers understand and avoid it.

 - popups the user asks for, such as (?) help windows, etc, are never
blocked.

The details of what I suggested are all targeted to solve the issue of
"how can we make browsing not annoying, but allow legitimate uses of
onload popups without making it difficult for the user". That's
exactly why I think the "popup blocked" icon should act to draw
attention to itself, but not be in-your-face like the popup (or 3
popups, as may be the case when visiting wsj.com for example) would be.

You don't even have to consider whitelists in this. The same UI would
work even if there was no whitelist, that's just a shortcut for making
the browser remember what you chose before, just like the password list.

PS: you can s/not annoying/safe/ above by considering kids and
accidentally ending up on a porn/etc site through a typo on google "I
feel lucky". Without effective popup blocking, you'll likely end up
with a dozen windows spewing images at you, opening new ones every
time you try to close an old one. Not a happy situation, and I've seen
it happen.

>The details of what I suggested are all targeted to solve the issue of
>"how can we make browsing not annoying, but allow legitimate uses of
>onload popups without making it difficult for the user".

Yeah, I think that's the right approach.
The delay thing seem to be really important to not make it difficult
for the user though, and I dont think it's implementable :/
With that popup blocking would not be that different from what Seth
proposed too (popup navigator idea). You can choose which popups to
allow and that is remembered too.
Unfortunately have to reload the page add an annoying extra step (at
least I'm assuming you need to do this in Phoenix right now ?).

Anyway we should probably try to implement this leaving it off by
default. If we will be able to improve it until it's not too
difficult, we will enable it.
Assuming someone has time to do it in 1.0 timeframe. I cant really
promise it :/
It's not hard though, the mozilla part is small and already in that
patch. There is also code for the icon in the statusbar and for the
white list. The remaining part is to deal with multiple popups blocked
(at least that patch seem to consider only 1 uri) and to write the
window from which popups are allowed.
A lot of people like popups blocking, so there is a chance someone
will do the work ;)

The only ui detail that really need to be solved is the accessible way
to access the blocked popups list (Dave, is the icon on the statusbar
an accessibility problem too ?)

i haven't read this thread here too in depth, but basically anything that isn't keynavigatable is an accessibility problem.

I'll vote for this. I just got spammed by popups, would be nice to see
this in Ephy, even if it's not perfect. Anything is better than
nothing here.

JIC people get the idea that spamming us with me toos will get this
  feature in epiphnay, well it won't work. So in the  future please don't
 me too :)

Ui proposals are appreciated though :)

*** Bug 107081 has been marked as a duplicate of this bug. ***

Marco,

I just glimpsed at the security page today and one thing that strikes
me odd is there is a blocking preference for "only this page" if i
remember correctly. What does this do? Are we using the right
terminology, since items that effect only a single page shouldn't be
in the prefs dialog.

That's cookies not popups. That means that if you are navigating
gnome.org, only gnome.org can set cookies, not the banners sites for
example. Apple use a bit different terminology, I just didnt want to
copy it, prolly you cant do it ;)
See it at http://www.codepoetry.net/archives/Safari-Security.php

I think we should use apple's terminlogy (or something pretty
similar), but without the help text. I'll add the help text to the
docs (where it belongs).

Sorry for the spam. Reassigning bugs with a target to our next milestone.

*** Bug 122430 has been marked as a duplicate of this bug. ***

* mpt says, having completely borked the original UI design for Mozilla
<daveb> mpt my idea, was block all by default
<mpt> ahem
<mpt> daveb: yep
<daveb> and have a menu item to enable per site
<mpt> yes, but
<daveb> not sure where it should go though
<mpt> also have a button in the status bar for opening the page's
popup windows
<guerrilla_> i like thatidea
<guerrilla_> =)
<mpt> And the first time it appears, have a balloon pointing to it
<mpt> (rather than Firebird's annoying dialog
<mpt> )
* daveb cut and pastes mpts comments into the relevant bug
<mpt> :-]
<daveb> mpt where would you put the menu item and what would it be
labelled?
<mpt> View menu, probably
<daveb> ok
<mpt> "Show Page's Popup Windows"

<guerrilla_> context based
<mpt> "Allow Site to Open Windows..."
<mpt> that bringing up a dialog so you can choose between
foo.bar.hum/baz, foo.bar.hum, and bar.hum (etc)
<mpt> (otherwise you end up allowing all from Geocities, which you
don't want, or conversely allowing all from .co.uk, which you don't
want either)
<mpt> guerrilla_: Making it Show/Hide would be interesting, but weird
<mpt> I suppose it might come in handy if a single page spawned a
dozen or more windows
<mpt> It would probably require a bit of architectural work ... each
window would need to have a linked list of all its parent windows
<guerrilla_> hmmm
<guerrilla_> difficult
<mpt> yup
<mpt> Whereas just "Show Page's Popup Windows" only requires a list of
windows which the current page has asked to open

Hacking the balloon would probably be hard ...
I wonder how this should interact with the pref. Considering that it's
off by default, is it worth to show the menu entry by default ?

well if we implement this correctly, i think there is probably a
pretty good argument that the pref should die and that popups should
always be blocked, unless specifically allowed. THe number of sites
using popups that decrease usability >>> the number of sites that use
them in a reasonable manner.

I'm not convinced we can enable it by default ... For how much easy
you make it, it's something that a newbie is going to have problems to
deal with. Noticing the button in statusbar, being able to allow the
popups from a list of sites, understand that he has to reload the page
to make it work again. IHMO there is enough to break the page for
several people. I think seth was against enabling by default too.

There is now an extensions for this in the Epiphany Extensions
package: http://ftp.gnome.org/pub/GNOME/sources/epiphany-extensions/ 

Target 1.2 -> 1.4 due to feature freeze.

Comment on attachment 16229 [details] [review]
Bleah

Marking obsolete attachments in open bugs.

Created attachment 28619 [details] [review]
Clone the Popup Blocker extension

I'll describe the user interface; this is a copy/paste from an email to Marco
(forwarded to Seth) a few days ago. It got Seth's approval so it can't be all
that bad:

----------
"Edit -> Preferences -> Privacy -> [ ] Allow popup windows" to set the
default popup acceptance behaviour.

"View -> [ ] Popup Windows" is the per-site preference. Unless this
preference has already been set for the site being visited, it will
follow the default preference.

When browsing to a page, if popup windows are disabled, a little icon
shows up in the statusbar. Its tooltip will read "x popups blocked"
where x is the number of popups. Doing "View -> [ ] Popup Windows" (to
enable it) will make the windows pop up. The statusbar icon will disappear.

If popup windows are enabled, doing "View -> [x] Popup Windows" (to
disable them) will make them disappear. The statusbar icon will return.
Choosing "View -> [ ] Popup Windows" again (to enable them again) will
put the windows in the same positions as they were before they were hidden.
----------

Problems with this patch:
- I realized just before submitting that I had also been hacking at something
else on the same checkout. So don't mind all the random "g_return_if_fail
(EPHY_IS_EMBED (embed))" stuff, I'll get rid of it before committing.
- I hacked ephy_embed_get_location() to use gtk_moz_embed_get_location() when
toplevel = TRUE. Refer to Bug #119047. This is necessary for popup blocking to
work, but I don't see why it shouldn't work that way anyway.
- Hidden popup windows shouldn't be able to use Javascript, so I put a function
in EphyBrowser, mozilla-embed and ultimately ephy_embed to enable/disable
javascript.
- The windows are opened with an evil hack:
ephy_embed_load_url("javascript:void(window.open(...));"). I can't think of a
better way to open windows, since we absolutely need the "features" argument.

Created attachment 28622 [details] [review]
Minor fix-ups of previous patch

Looked over it, fixed a memory leak and removed a tiny bit of cruft.

>- I hacked ephy_embed_get_location() to use gtk_moz_embed_get_location() when
>toplevel = TRUE. Refer to Bug #119047. This is necessary for popup blocking to
>work, but I don't see why it shouldn't work that way anyway.

See my comment on that bug. I think something like that is the way to go.

>- Hidden popup windows shouldn't be able to use Javascript, so I put a function
>in EphyBrowser, mozilla-embed and ultimately ephy_embed to enable/disable
>javascript.

Does mozilla/firefox do this too ?

>- The windows are opened with an evil hack:
>ephy_embed_load_url("javascript:void(window.open(...));"). I can't think of a
>better way to open windows, since we absolutely need the "features" argument.

How mozilla/firefox does this ?

>>- Hidden popup windows shouldn't be able to use Javascript, so I put a function
>>in EphyBrowser, mozilla-embed and ultimately ephy_embed to enable/disable
>>javascript.
>
>Does mozilla/firefox do this too ?

No. Firefox doesn't "hide" windows at all. It only opens them.

Easy justification for my "hiding" behavior: Let's say somebody browses with
"Allow Popup Windows" enabled and ten popups show up on a page. Go to "View ->
[x] Popup Windows" and they'd all disappear. Firefox doesn't do that.

>>- The windows are opened with an evil hack:
>>ephy_embed_load_url("javascript:void(window.open(...));"). I can't think of a
>>better way to open windows, since we absolutely need the "features" argument.
>
>How mozilla/firefox does this ?

It doesn't. When you click 'Unblock Site' it adds an entry to the permission
manager and that's *it*. You have to manually refresh the page to view the popup
windows.

Firefox's popup blocker seems stupid to me. Several flaws come to mind right away:
- There's no easy way to re-block a site after unblocking (or if blocking is
disabled by default). Personally, if I mistakenly unblock a site I just *leave*
it unblocked, since it's way too much hassle to re-block it.
- You have to click 'Refresh' after unblocking a site yet this is explained
nowhere (nor does it make sense from the user's perspective)
- The icon is meaningless (an "i" in a blue circle)
- After unblocking the site, before refreshing, the icon is *still* there, and
you can *still* open the "Blocked Popups" window and the "Unblock Site" button
is *still* clickable.
- Should an icon in the statusbar really be clickable? At the very least there
should be an alternate UI, since that one's not accessible.

However, right now Firefox has one thing my popup blocker doesn't: the first
time it blocks a site a dialog comes up explaining what's happening. This is
useful since the popup blocker icon can easily remain unseen (at the same time
it's stupid because it's supposed to *block* popup windows, not show its own). I
think my popup blocker needs some extra notification. For example, the tooltip
showing up for a couple of seconds on popup block.

To follow up on myself: Mozilla is adding support for opening single popups
(through nested submenus, ick). Here's the code *they* use to open it (Mozilla
bug #198846):

function popupBlockerMenuCommand(target) {
  var uri = target.getAttribute("uri");
  if (uri) {
    window.open(uri, "", target.getAttribute("features"));
  }
}

... that's basically what I do!

However, marco, if you're up for it, I could put an "EvaluateJS" function into
EphyBrowser so that we wouldn't need to use a javascript:void() hack. This is
how I was doing the patch originally, but nsIScriptGlobalContext.h (I think) was
conflicting with nsEmbedString.h so I undid all those changes.

A few comments after a quick glance at the patch:

- you enable js, but don't restore the original setting afterwards

- to enable js, you use the docshell -- should use nsIWebBrowserSetup instead
[http://www.mozilla.org/projects/embedding/embedapiref/embedapi10.html]

- the get_location problem will be taken care of in bug 119047

+	s->priv->popup_blocker_icon = gtk_image_new ();
+
+	gtk_image_set_from_stock (GTK_IMAGE (s->priv->popup_blocker_icon),
+				  EPHY_STOCK_POPUPS, GTK_ICON_SIZE_MENU);

Why not use gtk_image_new_from_stock ?

+		gtk_widget_show_all (statusbar->popup_blocker_frame);
Better use only show() and show the sub-widgets when creating them in
create_statusbar_popup_blocker_icon.

+	popup = g_new0 (BlockedPopup, 1);
+
+	popup->window = window;
+	popup->x = -1;
+	popup->y = -1;
+	popup->url = NULL;
+	popup->features = NULL;

Don't need to set them to NULL, since you used g_new0 already.

------

The problem with nsIScriptContext was that it includes nsString.h... if we
cannot find another way to evaluate JS in the context of a page, we should file
a mozilla bug for it.

> - to enable js, you use the docshell -- should use nsIWebBrowserSetup instead
> [http://www.mozilla.org/projects/embedding/embedapiref/embedapi10.html]

"The nsIWebBrowserSetup interface allows certain properties of a new browser
object to set before the browser window is opened"

However, I enable/disable Javascript on the fly. I don't doubt I do it the wrong
way -- I came up with that code myself, after all -- but are you sure
nsIWebBrowserSetup is the right way?

As for the rest of your comments: I'll work on them.

nsIWebBrowserSetup works just as well after the browser has been opened. And it
uses exactly the same code as your SetEnableJavascript, see
http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsWebBrowser.cpp#773
, and it's public API while the docshell is private (and will never be frozen).
Unfortunately, it does only allow setting the property and has no getter, so to
save the setting, we'll still need the docshell code... so maybe we can keep it
for setting the property, too :/

>To follow up on myself: Mozilla is adding support for opening single popups
>(through nested submenus, ick). Here's the code *they* use to open it

FWIW this is what I was asking about. I was not criticizing your design in
respect to firefox ... It's just that if you do something mozilla doesnt in most
cases your are going to be damned forever (see zoom persistance).

I have some nitpicks about the code but let's start from the base issues:

- Opening windows. I think you can just use nsIWindowWatcher, it takes a
features argument.
- Javascript blocking. That seem to be necessary because we hide the windows.
What was the reason to hide them instead of just closing (and reopening like in
the normal block case) ?

> - Javascript blocking. That seem to be necessary because we hide the windows.
> What was the reason to hide them instead of just closing (and reopening like in
> the normal block case) ?

Take this example situation:

Someone got a popup window and clicked on a link or moved or resized it. Then he
goes View -> Popup Windows on the original -- and the popup disappears. He wants
it back, and he wants it in the exact same state as it was before.

Not to mention, it would be annoying to have to wait for the page to load when
it has already been loaded.

Created attachment 28893 [details] [review]
Cleaner, taking Marco/Christian's advice

Changed patch according to recommendations.

Improvements:
- Fixes and uses Marco's new ephy_embed_single_open_window()
- Closes windows completely when blocking
- Uses gtk_image_new_from_stock()

Regressions:
- Can no longer close-and-then-open popup windows. I personally really like
that aspect of the UI. I just need to figure out the best way to open a window
given x, y, width, height and EphyEmbedChrome. I'll implement it with a third
GSList in tab->priv.

Notes:
- I left in my ephy_embed_get_location() hack; otherwise the entire patch
doesn't work.

The mozilla part looks good, expect you should use "address" not "location" as
signal argument. Please check the embed/* part expect the get_location hack.

I cant type s/expect/except

Crap let's retry.

The mozilla part looks good, except you should use "address" not "location" as
signal argument. Please check in the embed/* part except the get_location hack.

Hope there are no more typo :P

Okay, checked in embed/

Created attachment 28904 [details] [review]
Use newly-committed embed/

This is the same patch, minus the changes to embed/ which I just checked in. I
also fixed gtk_widget_show_all() in ephy-statusbar.c, which I had overlooked
from Christian's comment #43.

Created attachment 28970 [details] [review]
Latest revision

This one is (almost) feature-complete. When disabling popups, their (active
embeds') EphyEmbedChromes and window sizes are recomposed into a Javascript
features string, which is reinserted into the blocked_popups list. The only
thing missing is position, which depends on bug #144896 -- and position isn't
very important (though it would be nice).

I didn't include the embed/ hack; but yes, this patch still depends on bug
#199047.

Please comment. As far as I can see, it's ready for review (though admittedly,
I can't see very far).

+void
+ephy_statusbar_set_popup_blocker_tooltip (EphyStatusbar *statusbar,
+					  const char *tooltip)

I'd modify this on the model of the security one.

void          ephy_statusbar_set_popups_state         (EphyStatusbar *statusbar,
                                                       gboolean hidden,
                                                       const char *tooltip);

Probably it would be better to have a generic API instead. States could be
identified by string ids ... but I dont require that yet ;)

In general I think it's better to call it popup manager since our design isnt
exactly a blocker.

[ Kudos to chpe forms warning, it just saved my life ]

+	GSList *blocked_popups;

hidden_popups

+ case PROP_BLOCKED_POPUP_COUNT:

HIDDEN

+		case PROP_POPUPS_ALLOWED:

Maybe "DISPLAY_POPUPS" ? Based on your description ...

+} BlockedPopup; /* Never been opened */

PopupInfo

+free_blocked_popup (BlockedPopup *popup)

popups_manager_free_info

+popup_blocker_insert (EphyTab *tab,

popups_manager_add. No need to expose the insert/append difference, it's just an
implementation detail.

+popup_blocker_remove_window (EphyTab *tab,

popups_manager_remove_window

+popup_blocker_insert_window (EphyTab *tab,

popups_manager_add_window

+static gboolean
+popups_allowed (EphyTab *tab)

ephy_tab_get_popups_displayed.

+static void
+set_popups_allowed (EphyTab *tab,
+		    gboolean allowed)

ephy_tab_set_popups_displayed

+static guint
+popup_blocker_count (EphyTab *tab)

popups_manager_n_hidden

+show_blocked_popup (BlockedPopup *popup

popups_manager_show

+show_all_popups (EphyTab *tab)

popups_manager_show_all

+hide_popup (EphyWindow *window,

popups_manager_hide

Please use consistent arguments for show/hide, I guess EphyTab

+	popup->features = g_strdup_printf
+		("width=%d,height=%d,menubar=%d,status=%d,toolbar=%d",

Please factor this code out to a function

popups_manager_new_tab_info (EphyTab) maybe

+popup_blocker_reset (EphyTab *tab)

popups_blocker_reset

Note that I have not fully reviewed the code yet, but it seem ok. I'll possibly
come with nitpicks in next iteration though ;)

Created attachment 29105 [details] [review]
The Next Iteration

Implemented all marco's suggestions -- awaiting review.

+static void
+popups_manager_hide (EphyWindow *window,
+		     EphyTab *parent_tab)
+{
+	EphyEmbed *embed;
+	PopupInfo *popup;
+	char *location;
+
+	embed = ephy_window_get_active_embed (window);
+	g_return_if_fail (EPHY_IS_EMBED (embed));
+
+	location = ephy_embed_get_location (embed, TRUE);
+	if (location == NULL) return;
+
+	popup = g_new0 (PopupInfo, 1);
+	popup->url = location;
+	popup->features = popups_manager_new_window_info (window);
+
+	parent_tab->priv->hidden_popups = g_slist_prepend
+		(parent_tab->priv->hidden_popups, popup);
+
+	gtk_widget_destroy (GTK_WIDGET (window));
+
+	g_object_notify (G_OBJECT (parent_tab), "blocked-popup-count");
+}

Looks like you could reuse popups_manager_add here instead of duplicating ?

The property blocked-popup-count property should be renamed to hidden too. Also
we should think more about what terminology to expose in the UI (the menu use
show/hide, the tooltip and the pref blocked). Maybe open a separate bug about
that once we landed this.

So the last issue is get_location. Where exactly current api doesnt work ?
Both popups_manager_hide and ephy_tab_set_popups_displayed seem to be called on
the actually loaded uri, not on the loading one.

Wow, what do you know, that get_location problem disappeared in between
iterations of my patch.

I'll clean up everything tonight.

Heh, yeah, I thought we solved that too at some point. Feel free to commit once
you have done those changes.

Yeeeeeee-haw.

Filed bug #145229 on terminology, and committed. This bug is now CLOSED!

Yay !

I think current position of the menu is not correct though. I think it should be
either in his own group, after the first one, or in the first group.

Another alternative is:

Toolbar
Bookmarks bar
Statusbar
------------
Popups
Fullscreen
------------

The first group acts on parts of the window, the second on windows itself. Maybe
a bit too abstract though.