90008 - unix: should remove the Reveal Location & Launch File buttons from download progress dialog

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[sairuh (rarely reading bugmail)]

bug 67001 won't be fixed soon, so we ought to remove the Reveal Location and
Launch File buttons in the download progress dialogs for unix builds.ie, for the
branch builds at least.

Comment 1

[sairuh (rarely reading bugmail)]

when bug 67001 is fixed, this can prolly be removed as a dep and resolved.

Comment 2

[benc]

It looks like 67001 is not going to be resolved anytime soon. Sigh.

Anyhow, I see these light up (not grey as described elsewhere), so we need to
grey them or remove them for UNIX. 

Comment 3

[benc]

has anyone decided how this should work for MacOS X? I'm using 2001-09-05-05,
and the button doesn't work.

Comment 4

[sairuh (rarely reading bugmail)]

benc, see bug 91871.

Comment 5

[sairuh (rarely reading bugmail)]

spam: over to File Handling. i have not changed the assigned developer [or the
other fields for that matter], so if anyone realizes that a bug should have a
more appropriate owner, go ahead and change it. :)

Comment 6

[Blake Ross]

-->law

Comment 7

[Bill Law]

->mozilla0.9.9

Plan to remove on Linux; please address MacOS X in another bug.

Comment 8

[André Dahlqvist]

Attachment: Proposed, untested, patch to remove the buttons.

Comment 9

[André Dahlqvist]

I can't test this patch right now because for some reason the download dialog
doesn't come up at all for me with current CVS (regardless of this patch). Will
try to test it later.

Comment 10

[timeless]

Comment on attachment 56608 [details] [diff] [review]
Proposed, untested, patch to remove the buttons.

You can't just remove the code, there are other platforms that use this code, like Windows and MacOS.  Even BeOS implements this.

Comment 11

[André Dahlqvist]

Oops, sorry. For some odd reason I thought I was in a unix-subdirectory:-) Any
hints on how this could be done to only affect UNIX?

Comment 12

[sairuh (rarely reading bugmail)]

nominating nsbeta1, if bug 67001 isn't fixed in time...

Comment 13

[Bill Law]

Working on this stuff next.

Comment 14

[Peter Trudelle]

nsbeta1+ per ADT triage team.  Bill, is there someone else who could do this/

Comment 15

[Bill Law]

Yes, it's not hard; just requires a minimum of xul/js expertise.

The simplest fix would be to add code to nsProgressDialog.js to test
navigator.platform for Unix (!mac && !windows) and hide those buttons.

I think it would be nice to go the extra step of wiring up the "reveal location"
button so that it opens a browser window on the parent folder.  Then the user
would have some chance of doing something with the file.

Comment 16

[timeless]

um. beos implements these features, don't use the wrong logic for the if. [i 
can't remember if QNX does, but it certainly could]

Comment 17

[Blake Ross]

-> me

Comment 18

[Bill Law]

Attachment: patch (which I had worked out when I owned this bug)

Blake, here's the work I did on this.

Comment 19

[Peter Trudelle]

ADT3 per ADT triage team.

Comment 20

[Blake Ross]

hm, I did it slightly differently in download manager, but since you've already
got a patch...

Comment 21

[Peter Trudelle]

Mass moving nsbeta1+/adt3 bugs assigned to Navigator team engineers out to
target milestone 1.0.1.  Please confine your attentions to driving down our list
of TM 1.0 bugs for beta.  Better to help, debug, or test one of them than fix
one of these.

Comment 22

[Bill Law]

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

Comment 23

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 24

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 25

[Boris Zbarsky [:bzbarsky]]

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

Comment 26

[R.K.Aa.]

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

Comment 27

[Pete Zha]

Shall we fix this in branch since branch still has this bug.

Comment 28

[Pete Zha]

Attachment: patch against MOZILLA_1_0_BRANCH branch

Just don't enable those two button after download.

Comment 29

[Pete Zha]

=>

Comment 30

[Philip K. Warren]

The runningOnUnix function needs to also check for "aix" in the 
navigator.platform string. The suggested patch for the branch only checks 
for "unix", "linux", and "sun".

Comment 31

[Jim Dunn]

Comment on attachment 91924 [details] [diff] [review]
patch against MOZILLA_1_0_BRANCH branch

ugh these patches are ugly, unless every unix platform does return
"unix".  If this is not the case, then we really need to make
this unix friendly and throw everything...
aix, hp-ux, tru64/osf, beos....

Comment 32

[John Keiser (jkeiser)]

Yeah, I'm a little queasy about checking specific platforms but I understand the
reason.  Is there a way we could actually check whether the Launch / Reveal
functionality will be heeded directly?  That would be more accurate if there was
such a thing.

I'm a little confused about this too, is this only for branch?  I don't see
anything that says this was fixed on trunk.

Comment 33

[Philip K. Warren]

I don't know if there is a better way to do this, but perhaps this could be 
done as a pref. Add something like this to all.js:

pref(general.show_launch_reveal_buttons, "true");
or
pref(general.enable_launch_reveal_buttons, "true");

And then turn it off in unix.js. Would this be a suitable alternative?

Comment 34

[Pete Zha]

jkeiser, Yes, this is only for branch. Because trunk doesn't have this dialog,
they are using download manager now.

Comment 35

[Pete Zha]

Attachment: patch (MOZILLA_1_0_BRANCH)

A new patch using pref setting to disable these two buttons

Comment 36

[John Keiser (jkeiser)]

Comment on attachment 92538 [details] [diff] [review]
patch (MOZILLA_1_0_BRANCH)

r=jkeiser, slick man, slick :)

Comment 37

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92538 [details] [diff] [review]
patch (MOZILLA_1_0_BRANCH)

Put a try/catch around getting the prefservice and the prefvalue, and default
enableButtons to 'true' in the JS.  With those changes, sr=bzbarsky

Comment 38

[Pete Zha]

Attachment: patch with bz's comments. (against MOZILLA_1_0_BRANCH)

Comment 39

[Pete Zha]

Comment on attachment 92543 [details] [diff] [review]
patch with bz's comments. (against MOZILLA_1_0_BRANCH)

carry r&sr

Comment 40

[Pete Zha]

Attachment: patch same as previou one but for Trunk(07/22)

Same as previou patch, just for trunk

Comment 41

[Pete Zha]

Since bz told me that download manager can be custom in preference. That's mean
this bug also happen on trunk if the user don't use download manager and just
use download progress dialog.

Comment 42

[Asa Dotzler [:asa]]

Comment on attachment 92551 [details] [diff] [review]
patch same as previou one but for Trunk(07/22)

a=asa (on behalf of drivers) for checkin to 1.1

Comment 43

[Christian :Biesinger (don't email me, ping me on IRC)]

// Activate reveal/launch buttons if we are enable them.

I can't understand this comment. 
Can you fix it? I suppose you want some other verb than "are"

Comment 44

[John Keiser (jkeiser)]

Comment on attachment 92551 [details] [diff] [review]
patch same as previou one but for Trunk(07/22)

r=jkeiser on the trunk too.  Not sure if we strictly *need* to r-r= and sr=.

Comment 45

[Pete Zha]

Comment on attachment 92551 [details] [diff] [review]
patch same as previou one but for Trunk(07/22)

carry [e-mail]'s sr since it do the same thing with previou patch. And
I will change the comments when check in. Should be like this "Activate
reveal/launch buttons if we enable them"

Comment 46

[Pete Zha]

checked in trunk

Comment 47

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 92543 [details] [diff] [review]
patch with bz's comments. (against MOZILLA_1_0_BRANCH)

Approval granted for branch.  Change mozilla1.0.1+ to fixed1.0.1 when checked
in.

Comment 48

[Pete Zha]

checked in branch(MOZILLA_1_0_BRANCH) yesterday

Comment 49

[sairuh (rarely reading bugmail)]

the buttons are now disabled, but not hidden. i presume that's the expected
behavior, yes?

tested with 2002.08.15.07-1.0 bits on linux rh7.2.

Comment 50

[sairuh (rarely reading bugmail)]

as with the branch build, Reveal Location and Launch File are disabled (but not
removed) during and after file has downloaded. vrfy'ing with 2002.09.19.08 comm
trunk on linux rh7.2.