The default bug view has changed. See this FAQ.

[BeOS] Allow Launch/Reveal and ShowLocation in Tracker

RESOLVED FIXED

Status

SeaMonkey
Download & File Handling
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: Daniel Furrer, Assigned: Sergei Dolgov)

Tracking

({fixed1.8.1.8})

Trunk
x86
BeOS
fixed1.8.1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

15 years ago
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.
cc'ing some beos developers
(Reporter)

Comment 2

14 years ago
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
(Reporter)

Comment 3

14 years ago
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 ;))
(Assignee)

Comment 4

14 years ago
Created attachment 115994 [details] [diff] [review]
Patch

Allows to open BeOS native file explorer for downloaded file folder
(Assignee)

Comment 5

14 years ago
changing component
Assignee: asa → sergei_d
Component: Browser-General → Download Manager
OS: other → BeOS
(Assignee)

Comment 6

14 years ago
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.
Attachment #115994 - Attachment is obsolete: true
(Assignee)

Comment 7

14 years ago
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
(Assignee)

Comment 8

14 years ago
Created attachment 116005 [details]
Patch for 3 files

patches downloadmanager.js, Makefile.in and downloadmanager.properties. Needs
existence of beos folder from previous attachment
Attachment #115997 - Attachment is obsolete: true
(Assignee)

Comment 9

14 years ago
PC-only
Hardware: All → PC
(Assignee)

Updated

14 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 10

14 years ago
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

? review
Attachment #116004 - Flags: review?(arougthopher)

Comment 11

14 years ago
Comment on attachment 116005 [details]
Patch for 3 files

please review
Attachment #116005 - Attachment is patch: false
Attachment #116005 - Flags: review+
(Assignee)

Comment 12

14 years ago
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
Attachment #116005 - Attachment is patch: true
Attachment #116005 - Flags: review?

Comment 13

14 years ago
Comment on attachment 116005 [details]
Patch for 3 files

oops, I need to review it first
Attachment #116005 - Attachment is patch: false
Attachment #116005 - Flags: review+ → review?(arougthopher)

Updated

14 years ago
Attachment #116005 - Flags: review?

Comment 14

14 years ago
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

14 years ago
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.
(Assignee)

Comment 16

14 years ago
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

14 years ago
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

14 years ago
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)
(Assignee)

Comment 19

14 years ago
Thaks for notice. Met similar problem with URI instead Path when fixed last Net+
bookmark break.
Will look into.
(Assignee)

Comment 20

14 years ago
something wrong with getting path from RDF in function getFileForItem(aElement) {}
it initializes localFile with URI intead path as result

Comment 21

14 years ago
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

This zip should not contain the CVS directory.
Attachment #116004 - Flags: review?(arougthopher) → review-

Comment 22

14 years ago
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
Attachment #116005 - Flags: review?(arougthopher) → review-

Comment 23

14 years ago
bug 206441 should probably fix the file: problem -> marking it as blocking this bug
Depends on: 206441

Updated

14 years ago
Depends on: 175881

Comment 24

14 years ago
Comment on attachment 116004 [details]
new mozilla/xpfe/components/download-manager/resources/beos/ folder in zip

need to check this again
Attachment #116004 - Flags: review- → review?

Comment 25

14 years ago
Comment on attachment 116005 [details]
Patch for 3 files

need to check this again
Attachment #116005 - Flags: review- → review?

Updated

14 years ago
No longer depends on: 206441

Updated

14 years ago
QA Contact: asa → timeless

Comment 26

13 years ago
Sergei, what is the status of this patch? it obviously hasn't been applied to
the 1.7a netserver build from bebits.com

Prog.
(Assignee)

Comment 27

13 years ago
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

13 years ago
Was this bug ever resolved?

Comment 29

13 years ago
Who is svakharia@atsva.com and why does he have the access to remove all of our
addresses from the CC: field?

Comment 30

13 years ago
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.
Product: Browser → Seamonkey
(Assignee)

Comment 31

12 years ago
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"
Attachment #116004 - Attachment is obsolete: true
Attachment #116004 - Flags: review?
(Assignee)

Comment 32

12 years ago
Comment on attachment 116005 [details]
Patch for 3 files

new version will follow
Attachment #116005 - Attachment is obsolete: true
Attachment #116005 - Flags: review?
(Assignee)

Comment 33

12 years ago
changing bug name.
Probably there is DUP somewhere
Summary: BeOS: "Show in Browser" should become "Show in Tracker" → [BeOS] Allow Launch/Reveal and ShowLocation in Tracker
(Assignee)

Comment 34

12 years ago
Created attachment 205485 [details] [diff] [review]
patch

enables launch and "native show" for BeOS

review request
Attachment #205485 - Flags: review?(thesuckiestemail)

Comment 35

12 years ago
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.
Attachment #205485 - Flags: review?(thesuckiestemail) → review+
(Assignee)

Comment 36

12 years ago
Comment on attachment 205485 [details] [diff] [review]
patch

asking second(?) review
Attachment #205485 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 37

11 years ago
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 ;-)
Attachment #205485 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 38

11 years ago
>>          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
(Assignee)

Comment 39

11 years ago
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
Attachment #205485 - Attachment is obsolete: true
(Assignee)

Comment 40

11 years ago
landed in trunk by timeless
2005-12-10 21:37

closing bug
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 41

10 years ago
this very useful functionality is missing in current branch (1.8.1* ?)
needs backporting.
wondering if new bug is required to open
(Assignee)

Comment 42

10 years ago
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

10 years ago
Comment on attachment 205502 [details] [diff] [review]
patch

I don't see why you can't apply it to the branch as-is.
Attachment #205502 - Flags: approval-seamonkey1.1.5+
(Assignee)

Comment 44

10 years ago
/cvsroot/mozilla/xpfe/components/download-manager/resources/downloadmanager.js,v  <--  downloadmanager.js
new revision: 1.41.20.1; previous revision: 1.41
done 
Keywords: fixed1.8.1.8
You need to log in before you can comment on or make changes to this bug.