Closed Bug 295540 Opened 15 years ago Closed 8 years ago

Multi-monitor: download "save-as" dialog remains on secondary monitor even if detached


(Core :: Widget: Win32, defect, critical)

Windows XP
Not set





(Reporter: martin, Assigned: bbondy)



(Keywords: polish)


(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

The file "save-as" modal dialog box doesn't appear to update its own position
when sections of the desktop are removed ("de-attaching" a second monitor, which
created additional desktop space that had been used to show firefox).

The symptom is that -- after selecting a "save link as" -- the user's firefox
window becomes unresponsive -- only beeping when clicked on (because "save-as"
dialog is modal, I'm guessing).

workaround: if a user just presses "enter", the file gets saved to whatever
directory/filename comes up as the default and is then accessible from the
download manager window (which is correctly showing on the visible desktop space).

The "save-as" dialog box doesn't seem to recognize the fact that the desktop
area has changed.

Reproducible: Always

Steps to Reproduce:
This sequence replicates the problem:
1.  start windows
2.  attach second monitor, modify desktop properties so monitor appears to
"left" of main monitor as a desktop extension
3.  start and use firefox on the new, "left" monitor.  download file.  file-save
dialog moved to "left" monitor, populated and closed.
4.  shutdown firefox.  detatch second monitor by modifying desktop properties.  
5.  restart firefox.  go somewhere and try to download something.
Actual Results:  
unresponsive firefox window (click-ding, click-ding)

Expected Results:  
the file "save-as" dialog should be re-positioned in to the visible desktop space.
Dupe of/Related to bug 245418?
Depends on: ContextMenuScreen
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060728 Firefox/

This bug is easily reproducable and terribly annoying if working on a laptop and switching between single/multiple monitors.

From my every day usage, the Open/Save dialogs appear to be the only ones affected by this.

Confirming on FF on WinXP Pro SP2.


    *   IMPORTANT: It's a serious problem for end users:
        -   Modal dialogs (like Save As) effectively *disable* FF
        -   The user is given no clue what the cause of the
            problem is; FF simply stops working.
        -   If the user is disconnected long term from the
            secondary monitor, they have no opportunity to fix it.
            They'll need help from an advanced user. who'll have
            to stumble on the right question ('was another monitor
        -   They can't use affected dialogs until it's fixed

    *   Not sure what other dialogs are affected. See bug 368746 for
        a similar problem with page info dialogs.

    *   Workaround: With the dialog open, press ALT+SPACE. The
        Windows window control menu (Restore/Move/Size/etc) will
        appear. Click Move, the move the dialog with the arrow
        keys on the keyboard.

    *   Reproduced on mine with secondary monitor to right of main

Severity -> Critical

(Not a dupe of bug 245418, which was about context menus.)
Severity: normal → critical
Ever confirmed: true
Summary: download "save-as" dialog not visible if second monitor is "dettached" → Multi-monitor: download "save-as" dialog remains on secondary monitor even if detached
This bug is not critical ("crashes, loss of data, severe memory leak")
Severity: critical → normal
Assignee: nobody → win32
Component: Download Manager → Widget: Win32
Product: Firefox → Core
QA Contact: download.manager → ian
Steve - The bug hangs Firefox for affected users with no workaround (or not one that 99% of users will figure out). If hang = crash, which it does in this context, it's "critical".

Given it only affects a small number of users (those using multi-monitor setups that have detached a monitor), but they are hit hard.  

To shorten this discussion, I'll set it to Major as a compromise and, if you agree with my reasoning above, just change it to Critical.
Severity: normal → major
Severity: major → critical
Duplicate of this bug: 378757
Please see comment #3 in bug 378757.
Duplicate of this bug: 444262
Duplicate of this bug: 487914
Assignee: win32 → nobody
Flags: blocking1.9.2?
QA Contact: ian → win32
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Keywords: polish
This bug also affects users (like me) who have an expanded desktop across two monitors at work.

1. Open firefox and move it to the 2nd screen of the expanded desktop.
2. Remote desktop into the machine from a single-monitor system.
3. Open a modal file chooser dialog in firefox.
4. The dialog will open "off-screen" and make firefox appear to have hung.
I failed to reproduce this on Firefox 2.0, 3.0, 3.5, and a recent mozilla-central nightly using the steps in comment 0. Unless the steps actually mean for one to physically disconnect the second monitor's cable from the back of the computer; but it sounds to me like just modifying the desktop properties is intended.
I tried to recreate this problem on the Windows XP machine that I originally had this problem on.  I am no longer able to reproduce this bug on that machine.

I think this defect has been indirectly fixed by a Windows or Firefox update.
Assignee: nobody → netzen
OK I've been playing around with reproducing this for about an hour and I have a consistent way to reproduce it.

Windows 7: The problem seems fixed, probably by Windows itself.  
I even tried the original steps and my new steps below with Firefox and could not reproduce on Win7.

Windows Vista: Did not try.

Windows XP SP3: 
Problem 100% reproducible with FF, FF5, and 2011-08-10 Nightly with the following steps: 
1. Setup a primary monitor and an extended monitor.
2. Open Firefox and move it to the primary monitor.
3. Right click on a link and select save as...
4. Move the popup save as dialog to the extended monitor.
5. Close the popup save as dialog by pressing cancel.
6. Optional: Verify if that you do step 3. It should open in the extended monitor.  Cancel out again. 
7. Go to Windows XP settings and disable the extended monitor.
8. Right click and save as the same link as in step 4. 
Result: The window is disabled because of the modal popup that you can't see.
Expected result: The window should open on the primary monitor. 
Note: If you do a step 9 and re-enable the extended monitor while firefox seems frozen, you will see the popup at the original location.
2 more additional important points I found: 

1. With the steps in Comment 13 you can also reproduce this with an open file dialog.   

2. This bug is equally reproducible with a single monitor if you set your resolution to something high like 1600x1200, move the window towards the bottom, and then adjust your resolution to 800x600.
Setting you as the reviewer since you've been doing the other filpicker related patch reviews.
Attachment #552379 - Flags: review?(jmathies)
Comment on attachment 552379 [details] [diff] [review]
Patch for file picker popups are shown in last visible position, but should not if those positions are now non visible.

Review of attachment 552379 [details] [diff] [review]:

::: widget/src/windows/nsFilePicker.cpp
@@ +119,5 @@
> +static UINT_PTR CALLBACK FilePickerHook(HWND hwnd, UINT msg,
> +                                        WPARAM wParam, LPARAM lParam) 
> +{
> +  // Define an arbitrary user defined message we will post back to ourselves
> +  static const int UWM_ENSUREVISIBLE = WM_APP + 14159;

Hmm, could we move this to nsWindowDefs.h w/a good comment maybe? I know that doesn't fit perfectly but for the time being it's better than burying it here. We have a few custom message defined there currently.

Independent of that, looks good.
Attachment #552379 - Flags: review?(jmathies) → review+
Moved user defined message and renamed to match the other user defined messages with MOZ_ prefix instead of UWM.  Marking as r+ since it was previously r+'ed by :jimm.
Attachment #555416 - Flags: review+
Comment on attachment 552379 [details] [diff] [review]
Patch for file picker popups are shown in last visible position, but should not if those positions are now non visible.

># HG changeset patch
># Parent 95de91ae34a5a0f456f016be2fd2d8c819ad7885
># User Brian R. Bondy <>
>Bug 295540 - File picker popups are shown in last visible position, but should not if those positions are now non visible.
>diff --git a/widget/src/windows/nsFilePicker.cpp b/widget/src/windows/nsFilePicker.cpp
>--- a/widget/src/windows/nsFilePicker.cpp
>+++ b/widget/src/windows/nsFilePicker.cpp
>@@ -93,36 +93,81 @@ int CALLBACK BrowseCallbackProc(HWND hwn
>     if (filePath)
>       ::SendMessageW(hwnd, BFFM_SETSELECTIONW,
>                      TRUE /* true because lpData is a path string */,
>                      lpData);
>   }
>   return 0;
> }
>+static void EnsureWindowVisible(HWND hwnd) 
>+  // Obtain the monitor which has the largest area of intersection 
>+  // with the window, or NULL if there is no intersection.
>+  HMONITOR monitor = MonitorFromWindow(hwnd, MONITOR_DEFAULTTONULL);
>+  if (!monitor) {
>+    // The window is not visible, we should reposition it to the same place as its parent
>+    HWND parentHwnd = GetParent(hwnd);
>+    RECT parentRect;
>+    GetWindowRect(parentHwnd, &parentRect);
>+    BOOL b = SetWindowPos(hwnd, NULL, 
>+                          parentRect.left, 
>+                , 0, 0, 
>+                          SWP_NOACTIVATE | SWP_NOSIZE | SWP_NOZORDER);
>+  }
>+// Callback hook which will ensure that the window is visible
>+static UINT_PTR CALLBACK FilePickerHook(HWND hwnd, UINT msg,
>+                                        WPARAM wParam, LPARAM lParam) 
>+  // Define an arbitrary user defined message we will post back to ourselves
>+  static const int UWM_ENSUREVISIBLE = WM_APP + 14159;
>+  if (msg == WM_NOTIFY) {
>+    LPOFNOTIFYW lpofn = (LPOFNOTIFYW) lParam;
>+    if (!lpofn || !lpofn->lpOFN) {
>+      return 0;
>+    }
>+    if (CDN_INITDONE == lpofn->hdr.code) {
>+      // The Window will be automatically moved to the last position after
>+      // CDN_INITDONE.  We post a message to ensure the window will be visible
>+      // so it will be done after the automatic last position window move.
>+      PostMessage(hwnd, UWM_ENSUREVISIBLE, 0, 0);
>+    }
>+  } else if (msg == UWM_ENSUREVISIBLE) {
>+    EnsureWindowVisible(GetParent(hwnd));
>+  }
>+  return 0;
> // Callback hook which will dynamically allocate a buffer large
> // enough for the file picker dialog.
>-static UINT_PTR CALLBACK FilePickerHook(HWND hwnd, UINT msg,
>-                                        WPARAM wParam, LPARAM lParam)
>+static UINT_PTR CALLBACK MultiFilePickerHook(HWND hwnd, UINT msg,
>+                                             WPARAM wParam, LPARAM lParam)
> {
>   switch (msg) {
>     case WM_INITDIALOG:
>       {
>         // Finds the child drop down of a File Picker dialog and sets the 
>         // maximum amount of text it can hold when typed in manually.
>         // A wParam of 0 mean 0x7FFFFFFE characters.
>         HWND comboBox = FindWindowEx(GetParent(hwnd), NULL, 
>                                      L"ComboBoxEx32", NULL );
>         if(comboBox)
>           SendMessage(comboBox, CB_LIMITTEXT, 0, 0);
>       }
>       break;
>     case WM_NOTIFY:
>       {
>         LPOFNOTIFYW lpofn = (LPOFNOTIFYW) lParam;
>+        if (!lpofn || !lpofn->lpOFN) {
>+          return 0;
>+        }
>         // CDN_SELCHANGE is sent when the selection in the list box of the file
>         // selection dialog changes
>         if (lpofn->hdr.code == CDN_SELCHANGE) {
>           HWND parentHWND = GetParent(hwnd);
>           // Get the required size for the selected files buffer
>           UINT newBufLength = 0; 
>           int requiredBufLength = CommDlg_OpenSave_GetSpecW(parentHWND, 
>@@ -158,17 +203,18 @@ static UINT_PTR CALLBACK FilePickerHook(
>             lpofn->lpOFN->lpstrFile = filesBuffer;
>             lpofn->lpOFN->nMaxFile  = newBufLength;
>           }
>         }
>       }
>       break;
>   }
>-  return 0;
>+  return FilePickerHook(hwnd, msg, wParam, lParam);
> }
> NS_IMETHODIMP nsFilePicker::ShowW(PRInt16 *aReturnVal)
> {
>   // suppress blur events
>   if (mParentWidget) {
>@@ -241,17 +287,19 @@ NS_IMETHODIMP nsFilePicker::ShowW(PRInt1
>     ofn.lpstrTitle   = (LPCWSTR)mTitle.get();
>     ofn.lpstrFilter  = (LPCWSTR)filterBuffer.get();
>     ofn.nFilterIndex = mSelectedType;
>     ofn.hwndOwner    = (HWND) (mParentWidget.get() ? mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : 0); 
>     ofn.lpstrFile    = fileBuffer;
>     ofn.nMaxFile     = FILE_BUFFER_SIZE;
>+                OFN_EXPLORER | OFN_ENABLEHOOK;
>+    ofn.lpfnHook = FilePickerHook;
>     // Handle add to recent docs settings
>     nsCOMPtr<nsIPrivateBrowsingService> pbs =
>     PRBool privacyModeEnabled = PR_FALSE;
>     if (pbs) {
>       pbs->GetPrivateBrowsingEnabled(&privacyModeEnabled);
>     }
>@@ -300,26 +348,25 @@ NS_IMETHODIMP nsFilePicker::ShowW(PRInt1
>     MOZ_SEH_TRY {
>       if (mMode == modeOpen) {
>         // FILE MUST EXIST!
>         ofn.Flags |= OFN_FILEMUSTEXIST;
>         result = ::GetOpenFileNameW(&ofn);
>       }
>       else if (mMode == modeOpenMultiple) {
>-                     OFN_EXPLORER | OFN_ENABLEHOOK;
>         // The hook set here ensures that the buffer returned will always be
>         // long enough to hold all selected files.  The hook may modify the
>         // value of ofn.lpstrFile and deallocate the old buffer that it pointed
>         // to (fileBuffer). The hook assumes that the passed in value is heap 
>         // allocated and that the returned value should be freed by the caller.
>         // If the hook changes the buffer, it will deallocate the old buffer.
>-        ofn.lpfnHook = FilePickerHook;
>+        ofn.lpfnHook = MultiFilePickerHook;
>         fileBuffer.forget();
>         result = ::GetOpenFileNameW(&ofn);
>         fileBuffer = ofn.lpstrFile;
>       }
>       else if (mMode == modeSave) {
>         ofn.Flags |= OFN_NOREADONLYRETURN;
>         // Don't follow shortcuts when saving a shortcut, this can be used
Attachment #552379 - Attachment is obsolete: true
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Depends on: 684506
You need to log in before you can comment on or make changes to this bug.