Prior to 11 Aug, we redirected all window.open calls. After 11 Aug, we don't redirect *any* window.open calls *even ones that don't specify any chrome restrictions or size*. We *still* fail to follow the pref that exists to allow users to determine what they want to happen (aka the happy medium). We should obey the pref and set it to 2 by default in all-camino.js user_pref("browser.link.open_newwindow.restriction", 2); 0 = divert all window.open calls according to user's SWM prefs 1 = don't divert any window.open calls 2 = divert window.open according to user's SWM pref unless the window.open specifies size or features
define redirect. Are you saying that we now open new windows all the time regardless of the SWM setting? I tested that and it works fine.
i believe this is the relevant code to copy in Ff: http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsContentTreeOwner.cpp#799 I wish i knew what it was supposed to do or i'd just put it in.
s/redirect/divert/ = send window.open calls to a new tab (or same tab) when SWM is enabled Gah, I had a bad testcase for plain window.open; vanilla ones *are* diverted. Sorry. So this is just about honoring the pref (and setting it to 2 by default when people enable SWM).
Created attachment 236104 [details] [diff] [review] Partial Fix This is a partial fix. It makes us respect values of 0 and 2 for the pref. I'm not sure how we'd go about fixing this entirely, since we'll actually have to detect window.open calls (instead of just looking for changed chrome or specified size/position like we do now). I'll let the community/hwaara decide if this is acceptable (possibly with a followup bug).
Comment on attachment 236104 [details] [diff] [review] Partial Fix I don't really know how any our swm code works; I think it's better if pink reviews the change directly.
I'm not really sure we should do this. Setting the pref to anything other than 2 will most likely break web apps that assume this behavior. Sure, caveat emptor, but should we really give users enough rope to hang themselves if we can avoid it?
oh, also, I don't see |respectWindowOpenCallsWithSizeAndPosition| declared in any header. This should generate a warning, no?
Some users really want a true *single* window mode, where normal-clicking on any link works as $DEITY intended--opening that link in the same window. Users who set the pref to 0 are also more likely to know/pick out links that will cause problems when the pref is set to 0 and know how to "fix"/prevent it. E.g., during the time from the point SWM landed until bug 338777 landed, 0 was our default behavior with SWM on, and I knew which sorts of links I could click on and which sorts I needed to disable SWM before clicking in order for things not to be broken. It's fairly easy *for the sorts of people who would set this pref to 0* to figure out the sorts of links that are problematic, but supporting this pref gives them the flexibility to obtain the behavior they desire. (It's a shame we didn't get all these pref behaviors for free...such is the life of embedding, I guess....)
Created attachment 236516 [details] [diff] [review] Patch I agree with smokey - there is a group of users who'll want this. This patch addresses the issue with |respectWindowOpenCallsWithSizeAndPosition| being improperly declared.
Since I'm one of the users that would use this regularly, I agree with Smokey and froodian as well. We should get this in.
Comment on attachment 236516 [details] [diff] [review] Patch sr=pink
Checked in on trunk and 1.8branch