User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050907 Firefox/1.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050907 Firefox/1.4 when right clicking on an image in a tab with only the image(like "http://www.mywebsite.com/image.jpg")to use the "save image as" function, sometimes the save location window does not open especially whan a lot of tabs are opened with different images. Reproducible: Sometimes Steps to Reproduce: 1.Go to an website with an image gallery like for example "http://www.freeimages.co.uk/galleries/space/planets/index.htm". 2.right click on each of the image to "open link in a new tab". 3.now select each tab and right click to use the "save image as" funtion and after choosing the location and saving the image click on the red "x" to close the tab. 4. repeat this with all the other opened tab. Actual Results: Sometimes the "save image as" function does not brings the "save location window" but this rarely happens. Expected Results: using the "save image as" function should have always open the save location window. Doing a refresh by using the "reload current page" button will afterward allow the "save location window" to open and thus permit the image to be saved. This does not happen on specific websites but on all types of websites.

I get the prolem on the third image I try to save. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050916 Firefox/1.4

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050917 Firefox/1.6a1 ID:2005091723 When this happens I see two errors appear in the JS Console: Error: searchStr has no properties Source File: chrome://browser/content/browser.js Line: 4478 Error: gContextMenu has no properties Source File: chrome://browser/content/browser.xul Line: 1

Regression between 1.9a1_2005090612 and 1.9a1_2005090620.

I see this in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050918 Firefox/1.4 too.

regressionwindow for Trunk : http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&filetype=match&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-06+11%3A30%3A00&maxdate=2005-09-06+20%3A30%3A00&cvsroot=%2Fcvsroot Ria, do you happen to have the regressionwindow for branch aswell (roughly) ? Just days is fine, it would save me a lot of time not having to try 100's of builds to find it.

WFM - Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050914 SeaMonkey/1.1a Downloaded over 30 images could not reproduce.

(In reply to comment #5) Sure, branch: 1.8b4_2005090605 - 1.8b4_2005090613.

thanks for branch: http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-06+05%3A00%3A00&maxdate=2005-09-06+13%3A00%3A00&cvsroot=%2Fcvsroot thinking of Bug 307153 -> cc Aaron Leventhal

reloading the image is the only workaround I found for this bug.

Couldn't ship with it, thus nominating for 1.8b5.

->aaron

I don't think this is bug 307153. That bug just restored code we orginally had before the checkin for bug 258285, but added some null checks. I can't really duplicate the results in this bug. Can someone who can duplicate the testcase in this bug try backing out various patches to see which one it really is?

Hey guys found another way to workaround this issue. When the save dialog fails to open, select the download manager window then reselect the browser window and try again. Its worked everytime for me so far.

I think we need to better understand what's going on here. MozQA folks, can you help us get a clean testcase for this?

1. load some images in consecutive tabs 2. select the left most tab with an image 3. right click, save image 4. close that tab (I used middle click) 5. right click, save image - no file picker so far this is 100% reproducable for me JavaScript error: chrome://browser/content/browser.xul, line 1: gContextMenu has no properties ###!!! ASSERTION: cannot get parentTop: 'parentTop', file d:/mozilla/dom/src/base/nsGlobalWindow.cpp, line 5184 1. load some images in consecutive tabs 2. select the right most tab with an image 3. right click choose save image 4. close that tab (I used middle click) 5. right click choose save image 6. if you get a file picker close the tab and repeat step 5 until no file picker (often the second time) JavaScript error: chrome://browser/content/browser.xul, line 1: gContextMenu has no properties WARNING: Weird, we're finalized with a null mJSObject?, file d:/mozilla/dom/src/base/nsGlobalWindow.cpp, line 1624 ###!!! ASSERTION: cannot get parentTop: 'parentTop', file d:/mozilla/dom/src/base/nsGlobalWindow.cpp, line 5184 Note that the assertion does not happen until you click back into the window. This problem exists if you use local images too. This is from a Firefox Trunk Debug build, http://anchorweb.org/Fx/.mozconfig Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050923 Firefox/1.6a1.

(In reply to comment #12) > I can't really duplicate the results in this bug. The bug is very obvious. This site with larger images for instance: http://www.nasa.gov/multimedia/imagegallery/ New profile, no theme installed, saving the second, third or fourth image will always fail. Smaller images can give a less obvious result.

(In reply to comment #16) > (In reply to comment #12) > > > I can't really duplicate the results in this bug. > > The bug is very obvious. This site with larger images for instance: > http://www.nasa.gov/multimedia/imagegallery/ > New profile, no theme installed, saving the second, third or fourth image will > always fail. Smaller images can give a less obvious result. Using the Images at the nasa site I was able to reproduce this bug just once by openening several of the images in new tabs, then folling the steps in comment 15.

I still can't duplicate those results. And I don't believe the patch I checked in would have caused this. Anyone here know how to back individual patches out and rebuild to see what broke it?

Major discovery. This only looks like a regression from me because I fixed it and then broke it again when I reversed part of the change from bug 258285. It has been broken for a while at least. It is broken in the 7/30 build which is way before I checked in bug 307153. Here's the picture I think we probably have: Prior to checkin of bug 258285 (8/16 and earlier): broken After checkin of bug 258285, but before reversal of SendFocusBlur focus suppression (8/17 - 9/6): works After reversal of SendFocusBlur focus suppression (9/7 - now): broken So in otherwords when I removed SendFocusBlur window suppression temporarily it fixed the problem. We had to put that back in because of bug 307153. We need to find the regression window from before 7/30 !!!

Works in 2/23 Broken in 7/29 (so splitwindow is not the culprit) Anyone have cycles to find the regression window between those dates?

It still seems to be working in a Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050720 Firefox/1.0+, no bug present.

Works in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050725 Firefox/1.0+ Broken in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050726 Firefox/1.0+

(In reply to comment #22) > Works in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) > Gecko/20050725 Firefox/1.0+ > > Broken in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) > Gecko/20050726 Firefox/1.0+ http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&filetype=match&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-07-25+05%3A00%3A00&maxdate=2005-07-26+07%3A00%3A00&cvsroot=%2Fcvsroot

The checkin that broke it was for bug 301435, which focuses the selected item in the download manager. I checked it in for Doron Rosenberg who wrote the patch. 2005-07-25 15:21 aaronleventhal%moonset.net mozilla/ toolkit/ mozapps/ downloads/ content/ downloads.js 1.47 15/0 Bug 301435. Focus breaks when item removed from download manager. Patch by Doron Rosenberg. r+a=mconnor That code was later moved out of downloads.js to richlistbox.xml in bug 306522: http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/richlistbox.xml#248 This bug can be fixed by removing that line or turning off "Show Download Manager when a download begins"

(In reply to comment #24) > This bug can be fixed by removing that line or turning off "Show Download > Manager when a download begins" To clarify, removing that line causes problems with keyboard navigation in the download manager. Specifically, when you alt+click on a link and the download manager opens with the new item selected, you can't down arrow anymore. So, still looking for the right fix.

I think Mats may have suggested we do something like this once before. I can't say it's low risk for branch, but it doesn't regress bug 307153 which is a good sign. That's the bug which regressed when we tried to remove those SetSuppressFocus calls. We may want to take my proposed fix for trunk and consider just using the wallpaper workaround mentioned by Mconnor for branch. Mconnor, care to post that?

I think Mike Connor was working on a band aid fix for this issue. At this late point in the game, we'd probably be more interested in the band aid for the branch instead of changing nsEventStateManager which is historically very fragile.

(In reply to comment #27) > I think Mike Connor was working on a band aid fix for this issue. At this late > point in the game, we'd probably be more interested in the band aid for the > branch instead of changing nsEventStateManager which is historically very fragile. I agree with that, but I wanted to post what I think the correct long term fix is so we know what the alternative is. We may at least want the ESM fix for trunk.

Comment on attachment 198189 [details] [diff] [review] ESM change, not low risk. Put in symmetrical focus unsuppressions for "SendFocusBlur window switch" & "SendFocusBlur window switch #2" I'd feel a lot better about this if I knew where the focus unsuppression happens in the "normal" case right now. Not in SendFocusBlur, it seems.

(In reply to comment #29) > (From update of attachment 198189 [details] [diff] [review] [edit]) > I'd feel a lot better about this if I knew where the focus unsuppression > happens in the "normal" case right now. Not in SendFocusBlur, it seems. > It happens when a window gets activated. In ESM's NS_ACTIVATE handling it clears out any focus supressions just to be safe. That's the only way this gets turned off. Unfortunately NS_ACTIVATE doesn't happen as much in the world of tabbed browsing.

Comment on attachment 198189 [details] [diff] [review] ESM change, not low risk. Put in symmetrical focus unsuppressions for "SendFocusBlur window switch" & "SendFocusBlur window switch #2" ok.

too risky for this late in the game.

still too risky this late in the game.

(In reply to comment #26) > I think Mats may have suggested we do something like this once before. For reference: bug 306076 "Combined Patch rev. 2": attachment 195006 [details] [diff] [review]. The patch looks good but I see a difference to what I suggested. You unsuppress "switch #2" before the EnsureFocusSynchronization() calls (which calls fc->SetFocusedElement()) so there could potentially be two calls to UpdateCommands(). I had the unsuppression after to avoid that. If it still fixes this bug to have them after I would prefer that, unless you see a problem with it. (sorry for the late review)

Also fixes bug 312227.

Comment on attachment 199799 [details] [diff] [review] Avoid double command updates and potential for disagreeing variables by using only one Brian, this is an impovement over what you already sr='d

Comment on attachment 199799 [details] [diff] [review] Avoid double command updates and potential for disagreeing variables by using only one Looks good. r=mats

Comment on attachment 199799 [details] [diff] [review] Avoid double command updates and potential for disagreeing variables by using only one I think it would be much less error-prone if you wrote and used a small stack-based object that automatically unsuppresses the focus controller when it goes away. Also, I assume the mLastFocusedWith change is not related to this bug.

(In reply to comment #39) > (From update of attachment 199799 [details] [diff] [review] [edit]) > I think it would be much less error-prone if you wrote and used a small > stack-based object that automatically unsuppresses the focus controller when it > goes away. Okay, that is better. > Also, I assume the mLastFocusedWith change is not related to this bug. Right.

New patch coming? I know, nag nag nag.... /be

Blake's going to take a look at this bug and Aaron's patch too.

(In reply to comment #42) > Blake's going to take a look at this bug and Aaron's patch too. Thank you. I'm not around much for the next few days.

I haven't compiled this yet, so there might be silly typos.

Comment on attachment 200583 [details] [diff] [review] updated updated patch +class nsFocusSuppressor { + void Suppress(nsIFocusController *controller, const char *reason) Might be worth documenting that the reason argument is assumed to outlive the instance of this class. r+sr=jst

Comment on attachment 200583 [details] [diff] [review] updated updated patch Brian, would you give this an after-the-fact look over?

Fix checked into MOZILLA_1_8_BRANCH and trunk. (along with a bustage fix)

Comment on attachment 200583 [details] [diff] [review] updated updated patch Much better, I've thought we should do something like this for awhile.