333607 - Download Manager: contentAreaUtils.js error when saving link target

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Oliver Saier]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1

1. Subscribe to the above feed
2. Select an item, right-click the "Install" link at the bottom of the page, and "Save Link Target"

Reproducible: Always

Actual Results:  
Nothing happens, and the console reports: 
Error: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]
Source file: chrome://communicator/content/contentAreaUtils.js
Line: 517

Expected Results:  
File is downloaded through TB's download manager

Left-clicking the link opens it in Firefox (my default browser)

Comment 1

[Scott MacGregor]

I think we are using xpfe's version of contentAreaUtils.js which is using obsolete (for thunderbird anyway) download prefs.

one solution could be to figure out how to use toolkit's version of this file. 

Comment 2

[Žiga Sancin]

Attachment: switch to toolkit

This seems to work.

Comment 3

[Žiga Sancin]

Attachment: switch to toolkit v2

Oops, I forgot to diff the changes in messageWindow.xul.

Comment 4

[Adam Guthrie]

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

Comment 5

[Scott MacGregor]

Comment on attachment 239230 [details] [diff] [review]
switch to toolkit v2

Hmm, I'm worried this is more complicated. contentAreaUtils.js is also loaded by xpfe\commiunicator\contentAreaContextOverlay.xul which is in turn loaded by thundebird's ab, 3-pane, and stand alone msg windows. 

Might take more clean up to get this to work.

Maybe we need to fork contentAreaContextOverlay.xul, make it a mail specific file like mozilla/suite did, and make that load global/content/contentAreautils.js.

Comment 6

[Žiga Sancin]

Hm yes, forgot about xpfe... I suppose it's the same situation as with contentAreaClick.js.

Comment 7

[Žiga Sancin]

Hm, why does thunderbird actually use this xul overlay file? The xul part of the code isn't used by Thunderbird; nsContextMenu.js and contentAreaCommands.dtd are loaded elsewhere. Did I miss something?

Comment 8

[Scott MacGregor]

(In reply to comment #7)
> Hm, why does thunderbird actually use this xul overlay file? The xul part of
> the code isn't used by Thunderbird; nsContextMenu.js and
> contentAreaCommands.dtd are loaded elsewhere. Did I miss something?


I see it get loaded in two places:

http://lxr.mozilla.org/mozilla/source/mail/base/content/messenger.xul#47
and
http://lxr.mozilla.org/mozilla/source/mail/components/addrbook/content/addressbook.xul#43

Are you thinking we can remove those two lines?

Comment 9

[Žiga Sancin]

Yes, I've been running Thunderbird in my local tree with those two lines commented out for some time now. I still have to check out litmus for some testcases (to prove that I didn't miss something).

Comment 10

[Scott MacGregor]

(In reply to comment #9)
> Yes, I've been running Thunderbird in my local tree with those two lines
> commented out for some time now. I still have to check out litmus for some
> testcases (to prove that I didn't miss something).
> 

nice. And the context menus for html elements in the message pane still work good? i.e. on an image, a link, etc.?

Comment 11

[Joe Sabash [:JoeS1]]

I would like to be able to use trunk builds as my everyday newsreader.
But this is a real showstopper since hardly a day goes by that I don't need to save an inline image. It's been 6 months now. Can we get a fix or a dependable work-around.
see bug 334544 for more details, this effects more that RSS feeds and 'save ink as'
 

Comment 12

[Scott MacGregor]

We might end up having to implement:

https://bugzilla.mozilla.org/show_bug.cgi?id=334544#c11

if we can't figure this out for thunderbird 2.

Comment 13

[Žiga Sancin]

Attachment: new patch

I've built a fresh build and the context menus seem to work.

Comment 14

[Žiga Sancin]

Attachment: mail-jar.mn (branch)

The mail-jar.mn file is a bit different on the branch.

Comment 15

[Adam Guthrie]

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

Comment 16

[Nir]

I'm using TB-3a1 under ( Linux version ) and getting this bug also.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This bug is set as target for Thunderbird 2.0. Ziga any news about your patches? If they are ready can you request a review?

Comment 18

[Scott MacGregor]

I was trying out this patch before the holidays and was having some problems. Every time I try to save an image, I get the following JS error:

JavaScript error: chrome://communicator/content/nsContextMenu.js, line 527: save
URL is not defined

the weird thing is, it sure looks like we are including contentAreaUtils.js from toolkit where this gets defined.

Comment 19

[Žiga Sancin]

Thanks for the feedback Scott. I'll test the patch again, but it worked in my local tree (I could save an image, save a link target, etc.). Have you tried with a clean build?

Henrik: Sorry if it hadn't been obvious, but I just wanted to get some feedback on the patch. XPFE to toolkit migrations are complicated and we sure want to avoid unnecessary regressions in this area.

Comment 21

[Scott MacGregor]

bisi, just for grins, I checked this patch into the trunk (it wasn't going to make the problem worse) to see if a clean nightly build would work better than my debug build was with the patch. And I'm still getting the same errors reported in comment 18. Could I be missing part of your patch? 

2007-01-09 11:38	scott%scott-macgregor.org 	mozilla/mail/config/mail-jar.mn 	1.52 	1/2  	Bug #333607 --> download manager: unable to save link target. Try out bisi's patch on the trunk.
2007-01-09 11:38	scott%scott-macgregor.org 	mozilla/mail/components/addrbook/content/addressbook.xul 	1.67 	0/2 
2007-01-09 11:38	scott%scott-macgregor.org 	mozilla/mail/base/content/contentAreaClick.js 	1.12 	9/1 
2007-01-09 11:38	scott%scott-macgregor.org 	mozilla/mail/base/content/messageWindow.xul 	1.30 	1/1 
2007-01-09 11:38	scott%scott-macgregor.org 	mozilla/mail/base/content/messenger.xul 	1.67 	0/1 

Comment 22

[Žiga Sancin]

Thanks for the trunk checkin; I've tested the latest trunk build this morning. I'm seeing the problem in the message pane (isContentFrame and SaveURL aren't defined; both in contentAreaUtils.js), but the message window gets it right - so it must be something with the message pane. Oops. :)

It looks like we also need to include contentAreaUtils.js in messenger.xul, but I won't be able to test this in my local tree until tomorrow.

Comment 23

[Scott MacGregor]

Attachment: additional fix

ah that did it. thanks bisi. This is working for me now!

Comment 24

[Scott MacGregor]

Attachment: complete patch rolled up for the branch

Comment 25

[Scott MacGregor]

this is on the branch and trunk now. Thanks bisi.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Flag not needed anymore.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070111 Thunderbird/3.0a1 ID:2007011103

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061121 Thunderbird/2.0b1pre ID:2007011103

Comment 29

[Žiga Sancin]

Follow-up: bug 368914.