helper application dialog has title issues when opening an unknown filetype

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
3 years ago

People

(Reporter: bugzilla, Assigned: neil)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

14 years ago
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>".
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.
OS: Linux → All
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.
(Reporter)

Updated

14 years ago
Hardware: PC → All
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
Created attachment 160005 [details] [diff] [review]
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
Assignee: file-handling → cbiesinger
Status: NEW → ASSIGNED
Attachment #160005 - Flags: superreview?(bzbarsky)
Attachment #160005 - Flags: review?(neil.parkwaycc.co.uk)
Target Milestone: --- → mozilla1.8alpha4
(Assignee)

Comment 5

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

Comment 6

14 years ago
Created attachment 160015 [details] [diff] [review]
Other titles that need fixing
(Assignee)

Updated

14 years ago
Attachment #160015 - Flags: superreview?(shaver)
Attachment #160015 - Flags: review?(jst)
Comment on attachment 160005 [details] [diff] [review]
patch

sr=bzbarsky.  Fun...
Attachment #160005 - Flags: superreview?(bzbarsky) → superreview+
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.
Attachment #160005 - Flags: approval1.8a4?

Comment 9

14 years ago
*** Bug 261551 has been marked as a duplicate of this bug. ***
Comment on attachment 160005 [details] [diff] [review]
patch

a=asa for 1.8a4 checkin. Time is short so please land this quickly.
Attachment #160005 - Flags: approval1.8a4? → approval1.8a4+
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
Assignee: cbiesinger → neil.parkwaycc.co.uk
Status: ASSIGNED → NEW
Target Milestone: mozilla1.8alpha4 → ---
(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.
(Assignee)

Comment 13

14 years ago
Ah, that would be because it should be document.title, not window.title...
(Assignee)

Comment 14

14 years ago
Created attachment 160176 [details] [diff] [review]
document.title fixes
Attachment #160015 - Attachment is obsolete: true
(Reporter)

Comment 15

14 years ago
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?
seems likely. I only changed the seamonkey impl.

Updated

14 years ago
Flags: blocking1.8a4?
don't bother checking aviary trunk. aviary branch (where this apparently works)
still needs to be landed on the trunk.
*** Bug 262496 has been marked as a duplicate of this bug. ***
(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.
*** Bug 262726 has been marked as a duplicate of this bug. ***

Comment 21

14 years ago
Created attachment 160955 [details] [diff] [review]
Patch to ff bookmarks

browser/components/bookmarks/content needs to be fixed too.
Attachment #160015 - Flags: superreview?(shaver)
Attachment #160015 - Flags: review?(jst)
(Assignee)

Updated

14 years ago
Attachment #160176 - Flags: review?(jst)

Updated

14 years ago
Attachment #160955 - Attachment description: Patch to fb bookmarks → Patch to ff bookmarks
Attachment #160955 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 160176 [details] [diff] [review]
document.title fixes

r=jst
Attachment #160176 - Flags: review?(jst) → review+
(Assignee)

Updated

14 years ago
Attachment #160176 - Flags: superreview?(shaver)
Attachment #160176 - Flags: superreview?(shaver) → superreview+
(Assignee)

Comment 23

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

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

Updated

14 years ago
Attachment #160955 - Flags: review?(neil.parkwaycc.co.uk)

Comment 25

14 years ago
Created attachment 161710 [details] [diff] [review]
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.
Attachment #160955 - Attachment is obsolete: true
Comment on attachment 161710 [details] [diff] [review]
document.title fixes to ff (v0.2)

r=vladimir@pobox.com for ff browser/toolkit patch
Attachment #161710 - Flags: review+

Comment 27

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

Requesting sr
Attachment #161710 - Flags: superreview?(shaver)

Comment 28

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

Comment 29

14 years ago
*** Bug 264025 has been marked as a duplicate of this bug. ***
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.
Attachment #161710 - Flags: superreview?(shaver)
Attachment #161710 - Flags: superreview+
Attachment #161710 - Flags: approval-aviary?

Comment 31

14 years ago
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
Attachment #161710 - Flags: approval-aviary?

Comment 32

14 years ago
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
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 33

14 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 34

14 years ago
Created attachment 163520 [details]
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

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

Comment 36

14 years ago
Created attachment 163551 [details] [diff] [review]
Fix for regression

Comment 37

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

Updated

14 years ago
Attachment #163551 - Flags: review?(mscott)

Comment 38

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

Updated

14 years ago
Attachment #163551 - Flags: review?(mscott)

Comment 39

14 years ago
Created attachment 163557 [details] [diff] [review]
Fix for regression updated

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

Updated

14 years ago
Attachment #163551 - Attachment is obsolete: true

Updated

14 years ago
Attachment #163557 - Flags: review?(mscott)

Comment 40

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

thanks for the quick turn around!
Attachment #163557 - Flags: review?(mscott) → review+

Comment 41

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

Updated

14 years ago
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED
*** Bug 267283 has been marked as a duplicate of this bug. ***
Verified FIXED on Windows XP build 2004-11-19-04, Seamonkey trunk.
Status: RESOLVED → VERIFIED

Comment 44

14 years ago
Needs to be relanded once aviary branch has landed on trunk
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: reland-irsn

Comment 45

14 years ago
Re-checked in both patches
Keywords: aviary-landing

Comment 46

14 years ago
Fixed
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED

Updated

14 years ago
Keywords: aviary-landing
Whiteboard: reland-irsn
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.