59603 - link in sidebar panel with target="_top" overwrites chrome

Status: VERIFIED FIXED

(SeaMonkey :: Sidebar, defect, P2)

Description

[Ilya Gulko]

Targeting a link in the sidebar to "_top" causes the linked page to take over 
the whole browser window, replacing all of the XP elements, including the 
navigation buttons and the sidebar itself.

Example page:
http://geekissues.org/about.php

1) Go to the page with Mozilla
2) Click on "As a Netscape 6 My Sidebar Item".
3) Go to the GeekIssues.org sidebar, wait for the page to load, and then click 
on a link.

Expected:
Page is loaded in the browser's "browsing area".

Actual:
As described above.

Comment 1

[Jacob Steenhagen]

http://developer.netscape.com/docs/manuals/browser/sidebar/index.html#creating
Says to use target='_content' to put things in the content window.

I do have to agree, though... adding _top should NOT get rid of the UI.

Comment 2

[shrirang khanzode]

spam : changing qa to sujay (New Sidebar QA)

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Is this still happening? I tested this with today's Mozilla build and did not
get the results, but maybe the page & sidebar applications were changed?

This sounds really, really bad. To me this seems like you could actually replace
the whole UI with a spoofed one that you would not be able to tell from the
original just by looking at it (load XUL that LOOKS like the normal browser but
does something nasty, like stealing your passwords!)

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Attachment: New testcase - sidebar that can overwrite chrome

Comment 5

[Mitchell Stoltz (not reading bugmail)]

This bug still exists - and it is a Bad Thing. The URL above will load the
attached sidebar. The second link on the sidebar will overwrite the browser
chrome. As Heikki said, this could be used to spoof the browser chrome. Let's
get this fixed - sidebar should not have access to _top as a target. CCing hyatt.

Comment 6

[David Hyatt]

reassigning.  _top should not do this.  EEek!

Comment 7

[selmer (gone)]

Agreed, nsdogfood+.  Dan, can this get targetted to m.9 or m.9.1?

Comment 8

[Dan M]

Found it. Sidebar panels which should be sandboxed aren't, and vice versa. So any 
generic sidebar has chrome privileges to reach out and swat the _top chrome (and 
sidebars you'd expect to be trusted, don't). Fix:

Index: sidebarOverlay.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/components/sidebar/resources/sidebarOverlay.js,v
retrieving revision 1.71
diff -u -r1.71 sidebarOverlay.js
--- sidebarOverlay.js	2001/05/02 22:41:50	1.71
+++ sidebarOverlay.js	2001/05/15 01:02:37
@@ -367,10 +367,10 @@
   if (typeof this.iframe == "undefined") {
     var content = this.get_content();
     if (this.is_sandboxed()) {
-      var unsandboxed_iframe = content.childNodes.item(1);
+      var unsandboxed_iframe = content.childNodes.item(2);
       this.iframe = unsandboxed_iframe;
     } else {
-      var sandboxed_iframe = content.childNodes.item(2);
+      var sandboxed_iframe = content.childNodes.item(1);
       this.iframe = sandboxed_iframe;
     }
   }

Comment 9

[matt]

r=matt
tested and patched my build

Comment 10

[Dan M]

.

Comment 11

[sujay]

Ilya, does this work for you now?

Comment 12

[sujay]

verified in 5/24 build.

Comment 13

[Ilya Gulko]

I'm not sure if this deserves a reopening, so I'll leave that up to more
knowledgeable folks. The previous behavior, which was definitely problematic,
has been fixed, and no longer happens when using 2001052504 under Win32.
However, there are several other issues which I'm not sure are being addressed
correctly.

1) Clicking a link targeted to _top causes it to open in the Sidebar frame.
While that seems logical, a lot of web designers may (as did I) expect it to
instead open in the current browser frame. This probably needs discussion.

2) While testing the bug, I noticed that clicking "Back" in the context menu of
the Sidebar actually caused the content area (biggest part of the window) to go
back instead of the Sidebar. Does this deserve a separate bug?

Comment 14

[sujay]

Ilya,

can you please file separate bugs for this two issues?

thanks.

Comment 15

[Dan M]

Ilya: 1) If you want to open a page in the browser area, use _content. All we've 
done here is contain the damage of using _top, which, as the designer of a 
sidebar, you'll quickly stop using because it's almost certainly not what you 
wanted. 2) The sidebar's context menu probably shouldn't have a "back" item. 
Clearly a different bug.