From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1a) Gecko/20020617 BuildID: 2002061707 Reproducible: Always Steps to Reproduce: 1. Go to the given URL. 2. Click on the Edit item in Mozilla Main Menu. Note that both Find items are enabled. 3. Click on the link named "Far1704.exe". Save File box appears. 4. Press Esc or click Cancel button. 5. Click on the Edit item in Mozilla Main Menu again. Actual Results: Both Find items are disabled now. There's no way to get them back in this window. Expected Results: Find items should remain enabled.

Confirmed, WinNT SP6a, build 2002061707

Same thing in Win98, build 2002062808

Demo URL got obsolete. Offering a new one.

the previous URL is link to a exe file. changing to the download page.

preliminary patch for this bug. test passed on Win2k_En.

This bug can also be reproduced in PC\Redhat Linux 7.3 and SPARC\Solaris 9.

Attachment #92534 [details] [diff] has been tested successfully on PC\Redhat Linux 7.3 and SPARC\Solaris 9.

Can bbaetz,darin,jkeiser,rpotts give the attachment #92534 [details] [diff] [review] the rv and sr?

As by comment #6, OS -> All

bz, you're more familiar with this code than I am.

Simford: what exactly is the underlying problem that this patch solves?

The old codes: - if (channel) { - try { - ctype = channel.contentType; - if (this.mimeTypeIsTextBased(ctype)) - this.isImage.removeAttribute('disabled'); - else - this.isImage.setAttribute('disabled', 'true'); - } - catch (e) {} the new codes: + try { + // Disable menu entries for images, enable otherwise + if (content.document && this.mimeTypeIsTextBased(content.document.contentType)) + this.isImage.removeAttribute('disabled'); + else + this.isImage.setAttribute('disabled', 'true'); + } + catch (e) {} The exact problem of "Main menu items get forver disabled after the invocation of Save File dialog" is that the variable "channel" has changed to new value when Save File Dialog is open. so the variable "ctype"(= channel.contentType;) would mislead the states of the Main menu items. After many tests, I found that the "content.document.contentType" can keep the states of menuitems exactly. so I use the new codes in stead of the old ones.

Right. So the issue is that this stuff fires even when no new document is being loaded in the window... The real solution, imo, is not to trigger the nsBrowserStatusHandler.js code in such cases. That will become critical once we support "content-disposition: attachment" for MIME types that we handle internally.

bz: could you spend a little time to rv the patch #92534?

I did. See comment 13. I think it's OK wallpaper but is fixing the wrong thing in the wrong place... If we decide we really want that patch, it should be changed to use document.__proto__ to access the contentType property, since pages can override the value of that property (and thus disable Find on them, in this case).

new patch for Comment #13. bz: How about this time?

with the new patch, mozilla will skip the judgement(of the state of Find menu) when downloading a file.

That flag is false unless we're loading text/html.... so that test should always succeed, no? What am I missing?

I think bz is right. OnLocationChange shouldn't be called at all when we end up downloading. It might be easy to fix that, it might be a lot of work. The useRealProgressFlag patch is okay, though, like the other code that depends on it, I don't think it'll deal well with xhtml, svg, mathml and other xml based documents. Add to that that we moved that flag into nsBrowserStatusFilter, so your last patch unfortunately won't work.

if the location hasn't been changed, then don't need to change the menu status.

jag and bz: pls take a look at patch 94890

> + if (channel&&this.locationChanged) { Some spaces arounf the "&&" please? What cases have you tested this on? This looks like a much more correct fix, but I'd still like to know why OnLocationChange is being called at all....

add spaces around &&. bz: this patch add a boolean variable to indicate whether the location changed or not during downloading. if changed, then we test the ctype(=channel.contentType) normally. otherwise, we skip the test and keep the menu state. In the situation described in this bug, OnLocationChange has not been called. The patch concernes OnLocationChange, just because the patch need an indicator.

bz: Could you please give a r= to the patch 94894? Thanks!

Comment on attachment 94894 [details] [diff] [review] patch with spaces sr=bzbarsky. Please add a comment to the onStateChange code that says that the reason we need it is that the document may be null in onLocationChange (eg a full-page plugin) so we may not be able to get a content-type there.

Thanks bz very much! Have a good vacation!

Comment on attachment 95343 [details] [diff] [review] patch with new comments Only adding some comments, carry forward sr= from last patch. seek r= for this patch

seek r=

Comment on attachment 95343 [details] [diff] [review] patch with new comments r=jag

Thanks jag!

confirmed fixed 2002082104/winXP

.