48465 - "local" Sidebar tabs only work in browser

Status: VERIFIED FIXED

(SeaMonkey :: Sidebar, defect, P2)

Description

[german]

Current state:
Search and Bookmarks tabs (possibly others too) have no effect when used from 
within mail, aim or address book. Yet they look visible, enabled and ready to 
use. This is bad.
Recommend:
- If an internet search from the sidebar is evoked from a non-broswer app we 
should open a new browser window as needed/if none is open with the results 
shown. The search results should visible there as well as in the sidebar panel.
- If a bookmark gets clicked on from the sidebar from a non-broswer app we 
should open a new browser window as needed/if none is open with the url of that 
bookmark loaded. This is similar to clicking on a link in mail and expecting it 
to open of course in a browser window

If we cannot do any of these, we should at least disable these tabs in non-
browser apps. This is not the preferred remedy so, as it intorduces a great 
deal of hard-to-understand inconsistency among sidebar tabs.

Comment 1

[johng]

nav triage team:
[nsbeta3+] Claudius says this used to work, adding "regression".

Comment 2

[Steve Lamm]

The international folder loads a remote datasource. Because it has no children 
when it is first created, RDF does not mark it as a container. I used to have a 
style to fake the folder look, but the style hack will no longer work because we 
are taking out the icons in the customize panel.

I am planning to fix this by adding an observer to the RDF datasource. When the 
observer comes across a remote folder like the international one, it will force 
the container attribute to be true.

Comment 3

[Steve Lamm]

whoops, I updated the wrong bug. I still need to investigate this one.

Comment 4

[Phil Peterson]

PDT downgrading to P2 and leaving [PDTP2] in status whiteboard

Comment 5

[scottputterman]

cc'ing mscott.

This is interesting, when I double click on one of my bookmarks when I'm in the 
mail window, it actually loads it in the message pane.  If I had a message 
selected and then do this, it keeps the header of the old message but leads the 
web page into the content part of the message.

Comment 6

[jglick]

Yeek. It should open the bookmark in a Navigator window and bring the Navigator 
window to the front.

Comment 7

[don]

Ugh.  This is some kind of hideous targeting problem.

Re-assign to Matt for now (don't panic).

Comment 8

[matt]

It looks like the search one is fixed but bookmarks is not.
The targeting does not work for internal so we need to put in code that
targets the browser.   The function loadURLInContent()in
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/components/search/resou
rces/search-panel.js
does just this.  There is some checking code before this that needs to be added
also but i think we can just copy this code.  I'm wondering if
we should just make this common code for all apps to use but it might
be to much changing before shipping .

Ben,  any comments?  Bug that fixed this problem 
http://bugzilla.mozilla.org/show_bug.cgi?id=32034

Comment 9

[don]

Let's fix this in RTM instead of beta 3.

Matt, better send Ben some explicit email on this one.  I'm not sure if he's
reading the bugzilla notifications.

Comment 10

[johng]

nav triage team:
note that we can not disable these tabs in other components (that is a new
feature that was cut earlier because it was hard), so all we can do is fix them.

Marking [RTM NEED INFO] to get a patch that fixes the remaining problem with
bookmarks.  Search and Buddy List works now, and What's Related fails gracefully
(gives you nothing to click on, and OK if messy anyway).

Comment 11

[matt]

Attachment: diff for bm-panel.xul and bm-panel.js

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

r=ben. 

Comment 13

[don]

PDT, please approve.

Comment 14

[Phil Peterson]

PDT marking [rtm need info] since only one reviewer is listed. This is also a
large enough patch that we'd like to validate this on the trunk before landing
on the branch.

Comment 15

[matt]

new patch since the last one was to long
using openTopWin() instead of other funtions.
Also covers case if all browser windows are closed.
We should add this to the search panel also at some point.

Comment 16

[matt]

Attachment: shorter patch that uses openTopWin()

Comment 17

[matt]

So, this is checked in the branch now.
It works on my build and i just downloaded aunix build and
it works on that.  When one of the ring masters wants me to do anything more
with this code in my tree let me know.

r=mcafee for new one
sr=ben

Comment 18

[don]

Can QA verify that this works correctly now on the trunk?

Comment 19

[shrirang khanzode]

looks ok on brach builds for today(win, mac,linux). But I am not sure about 
why the search results are not getting updated in mail on every search or why 
sidebar bookmarks in  mail(mac) open inside the mail window. So am qa'ing 
claudius to verify this again on branch. To get this verified on trunk, pls add
keyword: vtrunk so that moz qa can verify this.

Comment 20

[matt]

ah, this was not checked in on the branch.  i checked the fix into the trunk.
The description shrirang gives is the bug.  I've tested it on unix on the branch
and it works.

Now that that is solved.
I a smaller patch to this bug suggested by rjc so there
a only a few diffences but PDT should be happy that
it's smaller. 
appoved by ben
Reviewed by rjc

Cases i've covered when testing.
Opening folders
Open bookmarks with double click
Opening bookmarks from mail and addressbook
Opening bookmarks from mailbook while all other windows are closed.
alt double click still pops up properties dialog
metakey double click opens new browser window.

Comment 21

[don]

Plus again.  We haven't checked it in othe branch.  Sorry for the confusion.

Comment 22

[matt]

Attachment: updated patch

Comment 23

[Phil Peterson]

marking [rtm++] since it's the only access to bookmarks in mail. Otherwise, I
think it wouldn't compare to the other crash/data loss/must fix bugs we're
taking this week.

Comment 24

[matt]

fixed

Comment 25

[sujay]

marking verified.