287459 - sidebar bookmarks don't load links without targets properly.

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[spacenuke]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2

I have a bookmark that opens a page in a sidebar that has a list of links. 
Clicking any of these links used to open the linked page in the main window up
to and including v1.0.1.  After upgrading to 1.0.2 today, my links started
opening in the sidebar window instead.

Reproducible: Always

Steps to Reproduce:
1.  create bookmark with "Load this bookmark in the sidebar" option checked.
2.  click on any link in the bookmarked page that opened in the sidebar.


Actual Results:  
the linked page will open inside the sidebar.

Expected Results:  
open linked page in main window.

Used to work in v1.0.1.

Comment 1

[Mike Connor [:mconnor]]

This works just fine for me.  Note that javascript: links should execute in the
sidebar properly now, where they did not before.

Please attach a testcase showing the "broken" behaviour and if warranted, I will
reopen the bug.

Comment 2

[spacenuke]

Bookmark the following page, then select "Load this bookmark in the sidebar."

http://www-personal.engin.umich.edu/~vtse/links/bug.html


Open the bookmark you've just created and click any of the links.  The linked
page opens in the sidebar instead of in the main window.  This wasn't the case
in 1.0.1.  Or is the 1.0.1 behavior the bug?

Comment 3

[Anbo Motohiko]

I can confirm this bug.
Mozilla/5.0 (Windows; U; Win 9x 4.90; ro-RO; rv:1.7.6) Gecko/20050318 Firefox/1.0.2

Pages with target="_content" work, but links in pages without target attributes
open in sidebar.
Maybe regression of Bug 284627 - arbitrary code execution via sidebar

Comment 4

[Mike Connor [:mconnor]]

WFM with a trunk nightly with the fix in.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050321
Firefox/1.0+

Comment 5

[Mike Connor [:mconnor]]

Broken on branch though.  In fact, the whole sidebar impl seems broken on branch.

Did context menus work on 1.0.1?

We may fix this if there's a 1.0.3, but that's not a guarantee.

Comment 6

[Mike Connor [:mconnor]]

For better or for worse, I own this.  Evaluate for 1.0.3 later.

Comment 7

[spacenuke]

The context menu within the sidebar doesn't work in 1.0.1.  The only menu item
that works is "View Page Source" which displays the html for the page that is in
the main window.

Comment 8

[Mike Connor [:mconnor]]

Note that I didn't change the target checking code, so there's no reason why
this should break.

1.0.x's sidebar impl seems to be generally hosed, current trunk context menus
works without issue.

Comment 9

[Kevin Gerich]

AFAIK the sidebar context menus have always been wonky but the link target bug
wasn't in 1.0.1. I have a sidebar with links that don't have target="_CONTENT"
that I use every day. Those links used to open in the main content area but
after upgrading to 1.0.2, they are spawning new windows. Middle-clicking the
links open them in new tabs in the main content area though.

Comment 10

[YUKI "Piro" Hiroshi]

I found the cause of this problem.

https://bugzilla.mozilla.org/attachment.cgi?id=176487
(from https://bugzilla.mozilla.org/show_bug.cgi?id=284627)
In the patch, a new function "webPanelSecurityCheck" is defined.
It calls another function "loadURI()" twice, but it is not defined
in the codes of 1.0 Branch. They should be "loadURL()".
("loadURI()" is for Trunk.)

Comment 11

[Anbo Motohiko]

Attachment: patch based on comment #10

Thank you, Hiroshi. I create a patch based on your comment.

This patch doesn't fix the context menu problem of sidebar (web panels). It's
another bug.
Bug 256577 - Context menu in sidebar not working when opening specified
website.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Anbo Motohiko:

You should request to review.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Brian:

This bug is regression of bug 284627.

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 178557 [details] [diff] [review]
patch based on comment #10

I'm surprised if this actually works, since the security manager expects an
nsIURI.  Better to port the makeURI function back to the branch.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

The patch actually works fine.

Comment 16

[Mike Connor [:mconnor]]

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

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 178557 [details] [diff] [review]
patch based on comment #10

ah, my bad, I had forgotten makeURL was just badly named before the 1.8 cycle. 
It actually does create a URI from a URL.

Comment 18

[Chase Phillips]

Comment on attachment 178557 [details] [diff] [review]
patch based on comment #10

Marking approval-aviary1.0.3-.	

Marking approval-aviary1.0.4+ assuming it gets sr=dveditz.  Hold off on landing
this until we release 1.0.3 and have the code tagged.

Comment 19

[Chase Phillips]

blocking-aviary1.0.3-
blocking-aviary1.0.4+

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 178557 [details] [diff] [review]
patch based on comment #10

a=dveditz, approval for 1.0.3 after all.

Comment 21

[Chase Phillips]

Comment on attachment 178557 [details] [diff] [review]
patch based on comment #10

Landed this on the Aviary1.0.1 branch on behalf of mconnor.

Checking in contentAreaUtils.js;
/cvsroot/mozilla/browser/base/content/Attic/contentAreaUtils.js,v  <-- 
contentAreaUtils.js
new revision: 1.34.2.2.2.12.2.3; previous revision: 1.34.2.2.2.12.2.2
done

Cleared approval-aviary1.0.4.

Comment 22

[Anbo Motohiko]

Fixed in 2005-04-04-16-aviary1.0.1.
Thanks all!

Comment 23

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change