Created attachment 202849 [details] [diff] [review] tentative patch I decided to use a whitelist instead of a blacklist, since there are possibilities of abuse by using combinations of jar: and view-source: etc that I didn't want to get into. I don't know whether this is the right way to go, and I need to test it more, but comments are welcome.
Assignee: dveditz → gavin.sharp
Status: NEW → ASSIGNED
Comment on attachment 202849 [details] [diff] [review] tentative patch Asaf pointed out that since this is only a safe-guard, trying to actively prevent people from shooting themselves in the foot is kinda silly.
Attachment #202849 - Attachment is obsolete: true
Created attachment 203679 [details] [diff] [review] patch This is probably better. I figured there should be some kind of indication for why the click failed, suggestions for improvements on that wording are welcome. This lets remote xul actually work, too.
Created attachment 203681 [details] [diff] [review] patch
Attachment #203679 - Attachment is obsolete: true
OS: MacOS X → All
Hardware: Macintosh → All
Attachment #203681 - Attachment is obsolete: true
Created attachment 203754 [details] [diff] [review] better patch After discussion with Jesse and Asaf, I think we all agreed that being able to open chrome, script, or data links using the text-link binding didn't really have a compelling use-case, and was possible by other means (onclick) for code that really did want to do it. This patch therefore disallows it entirely. This also uses checkLoadURI against "about:blank" instead of scheme comparisons, so it is more robust (handles nested protocols, for example).
Attachment #203748 - Attachment is obsolete: true
Created attachment 203756 [details] better testcase
Attachment #203682 - Attachment is obsolete: true
Created attachment 207358 [details] [diff] [review] better patch v2
Comment on attachment 207358 [details] [diff] [review] better patch v2 r=me with a comment explaining when and why its safe to use win.open(href)
Attachment #207358 - Attachment is obsolete: true
Attachment #207359 - Attachment is obsolete: true
(In reply to comment #12) > (From update of attachment 207358 [details] [diff] [review] ) > r=me with a comment explaining when and why its safe to use win.open(href) If newURI(href) fails (which is the only case where we'd reach the else block, other than for non-chrome), then window.open(href) shouldn't be an issue.
Whiteboard: [sg:want P3] [patch-r?] → [sg:want P3]
mozilla/toolkit/content/widgets/autocomplete.xml; new revision: 1.53;
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Attachment #210665 - Flags: branch-1.8.1?(mconnor)
Attachment #210665 - Flags: approval-branch-1.8.1?(mconnor) → approval-branch-1.8.1+
mozilla/toolkit/content/widgets/text.xml; new revision: 126.96.36.199;
Target Milestone: --- → mozilla1.8.1
hey Gavin, this patch broke text boxes in thunderbird. It looks like http urls are no longer getting sent to the desktop but instead are trying to get opened from a window.open call which has no meaning in thunderbird (there is no navigator.xul to open). See: http://forums.mozillazine.org/viewtopic.php?t=387499 for more details.
(In reply to comment #17) > hey Gavin, this patch broke text boxes in thunderbird. Is there a bug filed on that issue? If there is, please assign it to me.
I filed Bug 328890.
(In reply to comment #20) > Does this fix need to land on the 1.8.0 branch? No, this bug was more of a precautionary measure rather than addressing any specific problems, so I don't think it should land on the branch. I also think it should be removed from the security group. It's been fixed for a while, and it doesn't contain any security-sensitive information (certainly no more than bug 315004, which is already open).
You need to log in before you can comment on or make changes to this bug.