Last Comment Bug 172785 - [BeOS] Allow Launch/Reveal and ShowLocation in Tracker
: [BeOS] Allow Launch/Reveal and ShowLocation in Tracker
Status: RESOLVED FIXED
: fixed1.8.1.8
Product: SeaMonkey
Classification: Client Software
Component: Download & File Handling (show other bugs)
: Trunk
: x86 BeOS
: -- normal with 1 vote (vote)
: ---
Assigned To: Sergei Dolgov
: QA timeless
Mentors:
Depends on: 175881
Blocks:
  Show dependency treegraph
 
Reported: 2002-10-05 10:11 PDT by Daniel Furrer
Modified: 2007-09-26 15:08 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (914 bytes, patch)
2003-03-01 13:57 PST, Sergei Dolgov
no flags Details | Diff | Splinter Review
Patch - fixed identation (857 bytes, patch)
2003-03-01 14:36 PST, Sergei Dolgov
no flags Details | Diff | Splinter Review
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip (3.46 KB, application/zip)
2003-03-01 15:25 PST, Sergei Dolgov
no flags Details
Patch for 3 files (1.36 KB, text/plain)
2003-03-01 15:28 PST, Sergei Dolgov
no flags Details
patch (1.95 KB, patch)
2005-12-10 08:29 PST, Sergei Dolgov
thesuckiestemail: review+
neil: superreview+
Details | Diff | Splinter Review
patch (1.96 KB, patch)
2005-12-10 14:07 PST, Sergei Dolgov
neil: approval‑seamonkey1.1.5+
Details | Diff | Splinter Review

Description Daniel Furrer 2002-10-05 10:11:24 PDT
User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.1b) Gecko/20020723
Build Identifier: Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.1b) Gecko/20020723

In the Downloadmanager there's a button called "Show in Browser". I think this
should become "Show in Tracker" (as in "Show in Explorer" on windows)


Reproducible: Always

Steps to Reproduce:
1. Tools > Download Manager

Actual Results:  
It shows the files location in Mozilla

Expected Results:  
It should show the folder which contains the file in a new tracker window.
Comment 1 Christian :Biesinger (don't email me, ping me on IRC) 2002-11-23 09:37:54 PST
cc'ing some beos developers
Comment 2 Daniel Furrer 2003-02-12 07:39:41 PST
Since nsLocalFile::Reveal() is already implemented for BeOS the only thing that
needs to be changed is 
http://lxr.mozilla.org/seamonkey/source/xpfe/components/download-manager/resources/downloadmanager.js

223         // on unix, open a browser window rooted at the parent
224         if ((navigator.platform.indexOf("Win") == -1) &&
225             (navigator.platform.indexOf("Mac") == -1) &&
226             (navigator.platform.indexOf("OS/2") == -1)) {

needs to become:

223         // on unix, open a browser window rooted at the parent
224         if ((navigator.platform.indexOf("Win") == -1) &&
225             (navigator.platform.indexOf("Mac") == -1) &&
___             (navigator.platform.indexOf("OS/2") == -1) &&
___             (navigator.platform.indexOf("BeOS") == -1)) {

and the text on the Button needs to be changed
Comment 3 Daniel Furrer 2003-02-28 13:49:17 PST
I just tested the changes in the .js-file I suggested in #2 and it is working as expected.(Well, nearly: A Tracker window with the directory containing the file is opened. It would of course be nice if the downloaded file would also be selected but I think I'll file a new bug on this as soon as this bug is fixed ;))
Comment 4 Sergei Dolgov 2003-03-01 13:57:15 PST
Created attachment 115994 [details] [diff] [review]
Patch

Allows to open BeOS native file explorer for downloaded file folder
Comment 5 Sergei Dolgov 2003-03-01 14:02:09 PST
changing component
Comment 6 Sergei Dolgov 2003-03-01 14:36:37 PST
Created attachment 115997 [details] [diff] [review]
Patch - fixed identation

There is still little "style" problem. Item in Download Manager menu should be
"Show in Tracker" and shortcut must be 'T', but it needs changes in makefile.in
and creating of BeOS subfolder in resource folder, as far as i understand.
Comment 7 Sergei Dolgov 2003-03-01 15:25:16 PST
Created attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip
needed to introduce BeOS-specific chrome for download manager
Comment 8 Sergei Dolgov 2003-03-01 15:28:38 PST
Created attachment 116005 [details]
Patch for 3 files

patches downloadmanager.js, Makefile.in and downloadmanager.properties. Needs
existence of beos folder from previous attachment
Comment 9 Sergei Dolgov 2003-03-01 15:47:24 PST
PC-only
Comment 10 Sergei Dolgov 2003-03-17 17:01:53 PST
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

? review
Comment 11 Paul 2003-03-17 17:03:29 PST
Comment on attachment 116005 [details]
Patch for 3 files

please review
Comment 12 Sergei Dolgov 2003-03-17 17:04:28 PST
Comment on attachment 116005 [details]
Patch for 3 files

requestee box don't allow me to put any name here, hoping review from
arougthopher@lizardland.net
Comment 13 Paul 2003-03-17 17:04:44 PST
Comment on attachment 116005 [details]
Patch for 3 files

oops, I need to review it first
Comment 14 Paul 2003-03-29 14:55:35 PST
Comment on attachment 116005 [details]
Patch for 3 files

This,

>> ifneq (,$(filter BEOS,$(OS_ARCH)))

should be

>> ifneq (,$(filter BeOS,$(OS_ARCH)))


Also, could you provide unified diffs next time (cvs diff -u)?	They are much
easier to read and deal with.
Comment 15 Paul 2003-03-29 17:14:51 PST
This does not seem to work, since, the mPath.get() in nsLocalFileUnix returns
"file://.....", which, BPath does not seem to understand.

For this reason, a patch should also be applied to nsLocalFileUnix, so that
InitCheck is called on the BPath, before it is used.
Comment 16 Sergei Dolgov 2003-03-29 18:29:06 PST
Sorry, Paul, didn't understand your last comment...and in order to test how it
works - all Stripzillas now on bebits, including compact netserver version,
include this patch, and it works in download manager. 
But sure, there is one problem - sometimes you need "reselect" file/row again to
activate "Show in Tracker" control on toolbar.
Second imperfectnes is fact that it only opens Tracker in proper folder, but
don't select file there.
Comment 17 Paul 2003-03-29 19:04:01 PST
This does work, if, I download a file, select it in the download manager, and
choose "Show in Tracker".  But, if I choose a file in the download manager that
was downloaded previously, the selected file's location is prepended with a
"file:/" URL.  When this is passed to nsLocalFileUnix's reveal() method, it
cannot open Tracker, because BPath does not understand the "file:/" URL.

To illustrate this:
- Download a file to your machine to /boot/home
- Highlight the file in the download manager
- The status bar shows the file location as /boot/home/filename
- Restart mozilla
- Open the Download Manager from the Tools menu
- Highlight the file you downloaded previously
- The status bar shows the file location as file://boot/home/filename
- If you try to "Show in Tracker" this time, mozilla will crash
Comment 18 Paul 2003-03-29 19:26:37 PST
Also, before this is checked in, the contents.rdf should be updated with the
correct localeVersion, which, is currently 1.4a (though, I don't know how much
this matters, but, the latest unix/contents.rdf has 1.4a)
Comment 19 Sergei Dolgov 2003-03-29 19:46:07 PST
Thaks for notice. Met similar problem with URI instead Path when fixed last Net+
bookmark break.
Will look into.
Comment 20 Sergei Dolgov 2003-04-07 06:06:20 PDT
something wrong with getting path from RDF in function getFileForItem(aElement) {}
it initializes localFile with URI intead path as result
Comment 21 Paul 2003-04-19 09:46:42 PDT
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

This zip should not contain the CVS directory.
Comment 22 Paul 2003-04-19 09:49:02 PDT
Comment on attachment 116005 [details]
Patch for 3 files

declining review based on comment 17 and comment 18.

Also, if you could post unified diffs, they are much easier to read (-u)
Thanks
Comment 23 Ian Neal (Away until 7th Aug) 2003-05-27 15:26:56 PDT
bug 206441 should probably fix the file: problem -> marking it as blocking this bug
Comment 24 Paul 2003-06-05 06:27:21 PDT
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

need to check this again
Comment 25 Paul 2003-06-05 06:28:08 PDT
Comment on attachment 116005 [details]
Patch for 3 files

need to check this again
Comment 26 Prognathous 2004-01-04 19:12:03 PST
Sergei, what is the status of this patch? it obviously hasn't been applied to
the 1.7a netserver build from bebits.com

Prog.
Comment 27 Sergei Dolgov 2004-01-05 03:28:46 PST
Last i remember, there was something missing in our solution here, so it didn't
work as expected.
Then Paul have set review request, but without reviewer target, so it's unclear,
who should review it. I will test those updated solution this week, and if it
works, i put review here, but it needs also superreview, IMHO, as changes happen
outside BeOS-port-only folders
Comment 28 Disreali 2004-07-18 16:58:24 PDT
Was this bug ever resolved?
Comment 29 Disreali 2004-07-19 06:08:16 PDT
Who is svakharia@atsva.com and why does he have the access to remove all of our
addresses from the CC: field?
Comment 30 Prognathous 2004-07-19 08:58:48 PDT
I don't know who svakharia@atsva.com is, but you don't need any special
privileges to remove or add people to the cc list. Surprisingly, more often than
not this system works.

Prog.
Comment 31 Sergei Dolgov 2005-12-10 08:19:30 PST
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

no need for that, now DLManager has neutral item
"Show File Location"
Comment 32 Sergei Dolgov 2005-12-10 08:20:21 PST
Comment on attachment 116005 [details]
Patch for 3 files

new version will follow
Comment 33 Sergei Dolgov 2005-12-10 08:22:43 PST
changing bug name.
Probably there is DUP somewhere
Comment 34 Sergei Dolgov 2005-12-10 08:29:56 PST
Created attachment 205485 [details] [diff] [review]
patch

enables launch and "native show" for BeOS

review request
Comment 35 tqh 2005-12-10 08:34:59 PST
Comment on attachment 205485 [details] [diff] [review]
patch

r=thesuckiestemail@yahoo.se

Can't test it as I don't have Seamonkey, but I know the underlying code should work as I canged nsLocalFileUnix to work with BeOS.
Comment 36 Sergei Dolgov 2005-12-10 08:52:38 PST
Comment on attachment 205485 [details] [diff] [review]
patch

asking second(?) review
Comment 37 neil@parkwaycc.co.uk 2005-12-10 13:23:46 PST
Comment on attachment 205485 [details] [diff] [review]
patch

It really sucks that there's no way to tell ahead of time if launch won't work :-(

> var gCannotLaunch = ((navigator.platform.indexOf("Win") == -1) &&
>                      (navigator.platform.indexOf("OS/2") == -1) &&
>-                     (navigator.platform.indexOf("Mac") == -1));
>+                     (navigator.platform.indexOf("Mac") == -1) &&
>+                     (navigator.platform.indexOf("BeOS") == -1));
A regexp test would be nice, although not so important if you do the next suggestion.

>         if ((navigator.platform.indexOf("Win") == -1) &&
>             (navigator.platform.indexOf("Mac") == -1) &&
>-            (navigator.platform.indexOf("OS/2") == -1)) {
>+            (navigator.platform.indexOf("OS/2") == -1) &&
>+            (navigator.platform.indexOf("BeOS") == -1)) {
This looks familiar ;-) Perhaps this would work using if (gCannotLaunch) {

>           file = file.QueryInterface(Components.interfaces.nsIFile);
I don't think this line is necessary, so it would be nice if you could check and arrange to delete it ;-)
Comment 38 Sergei Dolgov 2005-12-10 13:44:36 PST
>>          file = file.QueryInterface(Components.interfaces.nsIFile);
>I don't think this line is necessary, so it would be nice if you could check
>and arrange to delete it ;-

Neil, I'm profanic in JS/XUL in general and in this given code aswell, also I lack permissions to checkin it myself, so only thing i can do here - submit patch with this line removed (under your responsibility) - to allow someone else to checkin this updated patch
Comment 39 Sergei Dolgov 2005-12-10 14:07:24 PST
Created attachment 205502 [details] [diff] [review]
patch

same as previous (so reviewed and superreviewed) but with change suggested by Neil:
line
file = file.QueryInterface(Components.interfaces.nsIFile);
removed
Comment 40 Sergei Dolgov 2005-12-11 04:21:24 PST
landed in trunk by timeless
2005-12-10 21:37

closing bug
Comment 41 Sergei Dolgov 2007-09-24 07:06:56 PDT
this very useful functionality is missing in current branch (1.8.1* ?)
needs backporting.
wondering if new bug is required to open
Comment 42 Sergei Dolgov 2007-09-24 07:26:20 PDT
Current patch with neil's sugestion about removing "QueryInterface" line 
https://bugzilla.mozilla.org/attachment.cgi?id=205502
applies absolutely cleanly also to branch.
Somwehat all this is also related to Bug 265025

So I'm waiting for Neil's comments about best method to apply it to branch, as with that removed QueryInterface line it affects also other besides BeOS platforms
Comment 43 neil@parkwaycc.co.uk 2007-09-24 07:46:24 PDT
Comment on attachment 205502 [details] [diff] [review]
patch

I don't see why you can't apply it to the branch as-is.
Comment 44 Sergei Dolgov 2007-09-26 15:08:33 PDT
/cvsroot/mozilla/xpfe/components/download-manager/resources/downloadmanager.js,v  <--  downloadmanager.js
new revision: 1.41.20.1; previous revision: 1.41
done 

Note You need to log in before you can comment on or make changes to this bug.