Camino fails to honor the pref for redirecting links



13 years ago
9 years ago


(Reporter: alqahira, Assigned: froodian)


({fixed1.8.1, testcase})

fixed1.8.1, testcase



(2 attachments, 1 obsolete attachment)

Prior to 11 Aug, we redirected all calls.  After 11 Aug, we don't redirect *any* 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("", 2);

0 = divert all calls according to user's SWM prefs
1 = don't divert any calls
2 = divert according to user's SWM pref unless the 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:

I wish i knew what it was supposed to do or i'd just put it in.
s/redirect/divert/ = send calls to a new tab (or same tab) when SWM is enabled

Gah, I had a bad testcase for plain; vanilla ones *are* diverted.  Sorry.

So this is just about honoring the pref (and setting it to 2 by default when people enable SWM).
Summary: Camino fails to honor the pref for redirecting links, and doesn't redirect any calls → Camino fails to honor the pref for redirecting links


13 years ago
Keywords: testcase

Comment 5

13 years ago
Posted patch Partial Fix (obsolete) — Splinter Review
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 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).
Attachment #236104 - Flags: review?(hwaara)

Comment 6

13 years ago
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.
Attachment #236104 - Flags: review?(hwaara) → review?(mikepinkerton)
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....)

Comment 10

13 years ago
Posted patch PatchSplinter Review
I agree with smokey - there is a group of users who'll want this.  This patch addresses the issue with |respectWindowOpenCallsWithSizeAndPosition| being improperly declared.
Assignee: nobody → stridey
Attachment #236104 - Attachment is obsolete: true
Attachment #236516 - Flags: review?(mikepinkerton)
Attachment #236104 - Flags: review?(mikepinkerton)
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]

Attachment #236516 - Flags: review?(mikepinkerton) → superreview+

Comment 13

13 years ago
Checked in on trunk and 1.8branch
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.