Closed Bug 344309 Opened 18 years ago Closed 18 years ago

PostMessage WM_QUIT to FirefoxMessageWindow does not quit the app on the trunk since Bug 326273 landed

Categories

(Firefox :: Shell Integration, defect, P1)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
Firefox 3 alpha1

People

(Reporter: robert.strong.bugs, Assigned: darin.moz)

References

Details

(Keywords: regression)

Attachments

(2 files)

The installer posts WM_QUIT to the window with the class of FirefoxMessageWindow to make the app quit. This works on the 1.8.1 branch but hasn't worked on the trunk since Bug 326273 landed. I suspect this affects other apps on the trunk but this has only been tested with Firefox.
Flags: blocking1.9a1?
Version: 2.0 Branch → Trunk
-> me (but let me know if this code is changing)
Assignee: nobody → darin
Severity: normal → critical
Priority: -- → P1
Target Milestone: --- → Firefox 3 alpha1
No plans on changing the PostMessage WM_QUIT call to AppShortNameMessageWindow and there shouldn't be anything special about it. Though this isn't related to this bug perhaps it would be a good thing to add a unique per app value for GWL_USERDATA on the windows with a class of MozillaUIWindowClass so they can be identified on a per app basis.
I'm not understanding.  Can you flesh-out the GWL_USERDATA idea more in a new bug?
ispiked used this to narrow down the regression window. I'll file a new bug re: GWL_USERDATA if I still think it is useful after thinking it through some more.
Attachment #228916 - Attachment description: NSIS exe that posts WM_QUIT in case it is useful → NSIS exe that posts WM_QUIT to FirefoxMessageWindow in case it is useful
Attached patch v1 patchSplinter Review
This patch does the trick.  When our event loop receives a WM_QUIT, we tell nsBaseAppShell::Run that it should stop looping and exit.  It's possible that nsIAppStartup::Exit should be called instead, but this patch basically restores the behavior to that of pre-threadmanager builds.
Attachment #228974 - Flags: review?(benjamin)
Comment on attachment 228974 [details] [diff] [review]
v1 patch

Can you file a bug on calling nsIAppStartup::Quit instead? That sounds like the right solution but requires some dependency thinking since nsIAppStartup is in tier 50.
Attachment #228974 - Flags: review?(benjamin) → review+
I think I forgot to mark this as fixed when I committed the v1 patch.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
FYI, bug 344440 covers using nsIAppStartup::Quit instead.
Flags: blocking1.9a1?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: