Closed Bug 344276 Opened 18 years ago Closed 18 years ago

uninstalling extension forecastfox will freeze firefox on quit

Categories

(Toolkit :: Add-ons Manager, defect, P1)

1.8 Branch
PowerPC
macOS
defect

Tracking

()

RESOLVED DUPLICATE of bug 347778
mozilla1.8.1beta2

People

(Reporter: jbecerra, Assigned: robert.strong.bugs)

Details

(Whiteboard: [has patch])

Attachments

(1 file, 2 obsolete files)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1

Uninstalling Forecastfox 0.9.2 (without disabling it first) will cause Firefox to hang when you Quit. You then have to force quit it to bring it back.

To recreate:
1. Install Forecastfox 0.9.2 and restart browser for the extension to start working
2. Open Add-ons option in the Tools menu and click on the Uninstall option for the extension
3. Quit the browser to restart and confirm that uninstall took place

Expected: Browser quits and you are able to restart it

Actual: Browser hangs and you have to right click on it's icon in the Dock to Force Quit it.

This does not happen with the Adblock extension, so it's probably extension specific.
Highly suspect Bug 344214 is the cause of this.
This is WFM on Win32 and Linux.

Zach, could you test this with a the latest 1.8.1 branch on Mac OS X?
Target Milestone: Firefox 2 beta2 → ---
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060802 BonEcho/2.0b1

I followed Juan's steps to reproduce but I didn't crash on restart after uninstalling the extension. However, Forecastfox remained installed for some reason. I reproduced this with both 0.9.2 and 0.9.3...
rs: I am seeing the same thing as Adam. No freeze on quit, but the extension does not uninstall after the restart. An examination of my profile dir showed that the Forecastfox dir remained in extensions/ and that nothing seems to have been deleted.
I would attach a log of what happens on shutdown but that contains at least 1000 assertions (the same one) and at least 10 warnings about "XPConnect wrapper thread use error"s.
Not sure why it is required to create the destination dir for Mac OS X and not other platforms.
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Flags: blocking-firefox2?
Severity: normal → critical
Priority: -- → P1
Target Milestone: --- → Firefox 2 beta2
Attached patch patch (obsolete) — Splinter Review
Benjamin, not sure what changed for Mac OS X and I don't have a Mac OS X dev environment to take this any further. This fixes it for me and with the freeze coming up I think this is the way to go especially with how safe this is.
Attachment #232458 - Attachment is obsolete: true
Attachment #232497 - Flags: review?(benjamin)
Whiteboard: [has patch][needs review bsmedberg]
Attachment #232497 - Flags: review?(mark)
Attachment #232497 - Flags: review?(benjamin)
Attachment #232497 - Flags: review+
Comment on attachment 232497 [details] [diff] [review]
patch

I will r+ this in light of the approaching code freeze, but the proper fix is in nsLocalFileOSX.cpp, nsLocalFile::MoveToNative:

594   nsCOMPtr<nsIFile> parentDir = newParentDir;
595   if (!parentDir) {
596     if (newName.IsEmpty())
597       return NS_ERROR_INVALID_ARG;
598     rv = GetParent(getter_AddRefs(parentDir));
599     if (NS_FAILED(rv))
600       return rv;   
601   }
      else {
        PRBool exists;
        rv = parentDir->Exists(&exists);
        if (NS_FAILED(rv))
          return rv;
        if (!exists) {
          rv = parentDir->Create(nsIFile::DIRECTORY_TYPE, 0777);
          if (NS_FAILED(rv))
            return rv;
        }
      }

Looks like I lost that feature in bug 294584, which only landed last week.  Are you sure that this patch fixes the problem as reported and not this more recent regression?
Attachment #232497 - Flags: review?(mark) → review+
(In reply to comment #8)
> (From update of attachment 232497 [details] [diff] [review] [edit])
> I will r+ this in light of the approaching code freeze, but the proper fix is
> in nsLocalFileOSX.cpp, nsLocalFile::MoveToNative:
What are the chances of getting this fixed on the 1.8.1 branch.

> Looks like I lost that feature in bug 294584, which only landed last week.  Are
> you sure that this patch fixes the problem as reported and not this more recent
> regression?
Definitely not sure since no one has been able to reproduce it as described in comment #0. :(
Whiteboard: [has patch][needs review bsmedberg] → [has patch]
Depends on: 347778
Filed bug 347778 for the problem as described in comment #8
No longer depends on: 347778
Checked in to trunk. I'll back this out after bug 347778 is fixed but it is safe with or without bug 347778 being fixed.

juan, if you could test this with a trunk build I'd appreciate it. Thanks

Leaving open until after verification from reporter
Comment on attachment 232497 [details] [diff] [review]
patch

Safe simple patch to workaround bug 347778 that is safe whether bug 347778 is fixed or not.
Attachment #232497 - Flags: approval1.8.1?
(In reply to comment #9)
> What are the chances of getting this fixed on the 1.8.1 branch.

Potentially serious regression, we should fix it.  I can't say whether or not it's possible before the b2 freeze, though.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060807 Minefield/3.0a1

I no longer see this problem, but testing on the trunk build might not be valid. Forecastfox 0.9.2 is not compatible with this version. And if I add and set the extension compatibility check to false, I get a different problem: upon restart, right after installation, the window is unresponsive. It won't let me close or quit; it only lets me quit with ctrl-click on the dock icon. On relaunch the extension works.

Forecastfox 0.9.3 works fine.
Comment on attachment 232497 [details] [diff] [review]
patch

Please review bug 347778 for 1.8.1 approval first since this patch shouldn't be needed if that one is approved.
Juan, you should file another bug for the issue you described in comment 14.
Per comment 15, we've taken bug 347778 and this becomes a duplicate against that.

*** This bug has been marked as a duplicate of 347778 ***
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
Comment on attachment 232497 [details] [diff] [review]
patch

Clearing request since we approved the other patch.
Attachment #232497 - Flags: approval1.8.1?
Backed out the patch from trunk
Flags: blocking-firefox2?
Attachment #232497 - Attachment is obsolete: true
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: