261267 - helper application dialog has title issues when opening an unknown filetype

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[sairuh (rarely reading bugmail)]

not sure who this should go to --please assign as needed. the helper application
dialog has title issues when opening an unknown filetype. found while testing
these builds on linux fedora core 2:

a. seamonkey (1.8a4, gtk1) 2004092307-trunk
b. firefox 2004092308-trunk

recipe:

1. open a link to a file which is an unknown type for your OS. for example, on
my linux system .BMP files are currently unknown:

http://mozilla.org/quality/browser/front-end/testcases/imaging/images/Summer.bmp

again, you'll need to test a with a file which will throw the resulting unknown
filetype (aka, "helper application") dialog.

2. look at the title for the resulting dialog.

expected: the title should say something like "Opening <filename>".

actual results:
a. in seamonkey, I see "Opening #1"
b. in firefox trunk bits, the title is blank.

Marcia/Tracy/et al., are you able to reproduce this on Windows? it might be more
tricky there, since many filetypes have already been defined by default there
(try looking for some wacky or rarely used extensions). I'll check this later on
on Mac OS X.

this was *not* an issue for the firefox (aviary1.0) branch bits
(2004092110-0.9+): when an unknown filetype is opened, the title of the dialog
says "Opening <filename>".

Comment 1

[Tracy Walker [:tracy]]

Confirmed on Windows Mozilla trunk build 2004-09-23-07-trunk. 
Using the Summer.bmp file supplied, I get the "Opening #1" in the Helper App
window Title Bar.

Comment 2

[Marcia Knous [:marcia]]

Confirmed on Mac using Mozilla 1.8a4
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a4) Gecko/20040923

I, too get the "Opening #1" in the Helper App window Title Bar.

Comment 3

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

ok, so I traced through the code. this.mTitle gets the correct value, then this
call happens:
 -  239          var win   = this.dialogElement( "nsHelperAppDlg" );
...
 -  262          win.setAttribute( "title", this.mTitle );

and it doesn't really seem to affect the title

Comment 4

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

Attachment: patch

this fixes this dialog, but there are others... most of
http://lxr.mozilla.org/seamonkey/search?string=setAttribute%28%22title needs to
be fixed, I suspect

Comment 5

[[e-mail]]

Comment on attachment 160005 [details] [diff] [review]
patch

>          this.mTitle = this.replaceInsert( win.getAttribute( "title" ), 1, fname);
The same trick applies when getting the title, which makes the "win" variable
redundant. r=me if you fix this too.

>-         win.setAttribute( "title", this.mTitle );
>+         this.mDialog.title = this.mTitle;

Comment 6

[[e-mail]]

Attachment: Other titles that need fixing

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 160005 [details] [diff] [review]
patch

sr=bzbarsky.  Fun...

Comment 8

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

Comment on attachment 160005 [details] [diff] [review]
patch

this fixes the title of the helper app dialog; since user see it when
downloading files, this should really work correctly in 1.8a4.

this should have very little risk, it just uses the newer way of setting the
title.

Comment 9

[Stefan Borggraefe]

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

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 160005 [details] [diff] [review]
patch

a=asa for 1.8a4 checkin. Time is short so please land this quickly.

Comment 11

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

helper app dialog patch checked in

Checking in embedding/components/ui/helperAppDlg/nsHelperAppDlg.js;
/cvsroot/mozilla/embedding/components/ui/helperAppDlg/nsHelperAppDlg.js,v  <-- 
nsHelperAppDlg.js
new revision: 1.59; previous revision: 1.58
done


-> neil for this patch to the other dialogs

Comment 12

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

(In reply to comment #5)
> The same trick applies when getting the title, which makes the "win" variable
> redundant. r=me if you fix this too.

hmm, I forgot to address this when checking in. However, that's a good thing,
because it doesn't seem to work - this.mDialog.title is empty.

Comment 13

[[e-mail]]

Ah, that would be because it should be document.title, not window.title...

Comment 14

[[e-mail]]

Attachment: document.title fixes

Comment 15

[sairuh (rarely reading bugmail)]

looks good with 2004092709-trunk seamonkey build on linux, the titlebar now says
"Opening <filename>". case (b) from comment 0 still occurs (empty titlebar), but
this is likely b/c the UI forked for this feature in firefox, right?

Comment 16

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

seems likely. I only changed the seamonkey impl.

Comment 17

[Hixie (not reading bugmail)]

don't bother checking aviary trunk. aviary branch (where this apparently works)
still needs to be landed on the trunk.

Comment 18

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

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

Comment 19

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

(In reply to comment #17)
> don't bother checking aviary trunk. aviary branch (where this apparently works)
> still needs to be landed on the trunk.

actually I believe this was caused by a trunk-only patch so the branch landing
will not fix this. but feel free to delay this until then, of course.

Comment 20

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

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

Comment 21

[Ian Neal]

Attachment: Patch to ff bookmarks

browser/components/bookmarks/content needs to be fixed too.

Comment 22

[Johnny Stenback (:jst)]

Comment on attachment 160176 [details] [diff] [review]
document.title fixes

r=jst

Comment 23

[[e-mail]]

Comment on attachment 160176 [details] [diff] [review]
document.title fixes

Checked in; is there a Firefox version of this bug for the remaining fixes?

Comment 24

[Steffen Wilberg]

> Checked in; is there a Firefox version of this bug for the remaining fixes?
Besides bug 262254, and the "ff bookmarks" patch in this bug, none that I know of.

Comment 25

[Ian Neal]

Attachment: document.title fixes to ff (v0.2)

List of files to be patched was given by Neil - if patched wrong then that's my
fault.

Comment 26

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 161710 [details] [diff] [review]
document.title fixes to ff (v0.2)

r=[e-mail] for ff browser/toolkit patch

Comment 27

[Ian Neal]

Comment on attachment 161710 [details] [diff] [review]
document.title fixes to ff (v0.2)

Requesting sr

Comment 28

[Steffen Wilberg]

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

Comment 29

[Erik Fabert]

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

Comment 30

[Mike Shaver (:shaver emeritus)]

Comment on attachment 161710 [details] [diff] [review]
document.title fixes to ff (v0.2)

sr=shaver on those bits as well.  Aviary approval would seem the order of the
day: requesting.

Comment 31

[Ian Neal]

Comment on attachment 161710 [details] [diff] [review]
document.title fixes to ff (v0.2)

Cancelling approval request as enhancement that caused this breakage did not
hit the branch

Comment 32

[Ian Neal]

Checking in browser/toolkit patch
Checking in browser/components/bookmarks/content/addBookmark.js;
/cvsroot/mozilla/browser/components/bookmarks/content/addBookmark.js,v  <-- 
addBookmark.js
new revision: 1.14; previous revision: 1.13
done
Checking in browser/components/bookmarks/content/bookmarksManager.js;
/cvsroot/mozilla/browser/components/bookmarks/content/bookmarksManager.js,v  <--
 bookmarksManager.js
new revision: 1.15; previous revision: 1.14
done
Checking in browser/components/bookmarks/content/bookmarksProperties.js;
/cvsroot/mozilla/browser/components/bookmarks/content/bookmarksProperties.js,v 
<--  bookmarksProperties.js
new revision: 1.17; previous revision: 1.16
done
Checking in toolkit/content/widgets/wizard.xml;
/cvsroot/mozilla/toolkit/content/widgets/wizard.xml,v  <--  wizard.xml
new revision: 1.13; previous revision: 1.12
done
Checking in toolkit/components/help/content/help.js;
/cvsroot/mozilla/toolkit/components/help/content/help.js,v  <--  help.js
new revision: 1.12; previous revision: 1.11
done
Checking in toolkit/content/fontpackage.js;
/cvsroot/mozilla/toolkit/content/fontpackage.js,v  <--  fontpackage.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/downloads/content/nsHelperAppDlg.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/nsHelperAppDlg.js,v  <-- 
nsHelperAppDlg.js
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in,v  <-- 
nsHelperAppDlg.js.in
new revision: 1.13; previous revision: 1.12
done
Checking in toolkit/mozapps/extensions/content/about.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/about.js,v  <--  about.js
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <-- 
extensions.js
new revision: 1.38; previous revision: 1.37
done

Comment 33

[Scott MacGregor]

this broke the helper app dialog in firefox and thunderbird on the trunk.

Opening the dialog generates the following error from the code added to
toolkit's nsHelperAppDialog.js:

JavaScript error: file:///C:/build/trees/dbg/mozilla/dist/bin/components/nsHelpe
rAppDlg.js, line 320: win.document has no properties

Comment 34

[Scott MacGregor]

Attachment: screen shot of the helper app dialog after updating to pick up this change

as you can see things look pretty broken but it's all because of the JS error
that keeps the rest of the onload stuff from happening.

Comment 35

[Scott MacGregor]

cc'ing bienvenu, he was also seeing this regression on the trunk builds. 

Comment 36

[Ian Neal]

Attachment: Fix for regression

Comment 37

[Ian Neal]

http://neil.rashbrook.org/pyeximon.xul
Works fine on nightly but not on 0.10.1 so something hasn't made it into the branch

Comment 38

[Ian Neal]

Ignore comment #37, I was in the wrong bug window

Comment 39

[Ian Neal]

Attachment: Fix for regression updated

Neil pointed out that it looks like the win variable is no longer in use, so
removing that too.

Comment 40

[Scott MacGregor]

Comment on attachment 163557 [details] [diff] [review]
Fix for regression updated

thanks for the quick turn around!

Comment 41

[Ian Neal]

Comment on attachment 163557 [details] [diff] [review]
Fix for regression updated

Checking in content/nsHelperAppDlg.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/nsHelperAppDlg.js,v	<-- 
nsHelperAppDlg.js
new revision: 1.9; previous revision: 1.8
done
Checking in src/nsHelperAppDlg.js.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in,v  <-- 
nsHelperAppDlg.js.in
new revision: 1.14; previous revision: 1.13
done

Comment 42

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

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

Comment 43

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on Windows XP build 2004-11-19-04, Seamonkey trunk.

Comment 44

[Ian Neal]

Needs to be relanded once aviary branch has landed on trunk

Comment 45

[Ian Neal]

Re-checked in both patches

Comment 46

[Ian Neal]

Fixed