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

RESOLVED FIXED in mozilla9

Status

()

Core
Widget: Win32
--
critical
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Martin Crundall, Assigned: bbondy)

Tracking

({polish})

unspecified
mozilla9
x86
Windows XP
polish
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 -
wanted1.9.2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
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?

Updated

12 years ago
Depends on: 245418

Comment 2

11 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

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.

Comment 3

10 years ago
Confirming on FF 2.0.0.3 on WinXP Pro SP2.

Notes:

    *   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
            attached?')
        -   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
        monitor.


Severity -> Critical

(Not a dupe of bug 245418, which was about context menus.)
Severity: normal → critical
Status: UNCONFIRMED → NEW
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

Updated

10 years ago
Assignee: nobody → win32
Component: Download Manager → Widget: Win32
Product: Firefox → Core
QA Contact: download.manager → ian

Comment 5

10 years ago
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

Updated

10 years ago
Severity: major → critical

Updated

9 years ago
Duplicate of this bug: 378757

Comment 7

8 years ago
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

Comment 10

8 years ago
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.

Comment 12

8 years ago
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)

Updated

6 years ago
Assignee: nobody → netzen
(Assignee)

Comment 13

6 years ago
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 2.0.0.3 and could not reproduce on Win7.

Windows Vista: Did not try.

Windows XP SP3: 
Problem 100% reproducible with FF 2.0.0.3, 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.
(Assignee)

Comment 14

6 years ago
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.
(Assignee)

Comment 15

6 years ago
Created 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.

Setting you as the reviewer since you've been doing the other filpicker related patch reviews.
Attachment #552379 - Flags: review?(jmathies)

Comment 16

6 years ago
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+
(Assignee)

Comment 17

6 years ago
Created attachment 555416 [details] [diff] [review]
Patch for file picker popups are shown in last visible position, but should not if those positions are now non visible. v2.

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+
(Assignee)

Comment 18

6 years ago
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 <netzen@gmail.com>
>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, 
>+                          parentRect.top, 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)
> {
>   NS_ENSURE_ARG_POINTER(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.Flags = OFN_SHAREAWARE | OFN_LONGNAMES | OFN_OVERWRITEPROMPT |
>-                OFN_HIDEREADONLY | OFN_PATHMUSTEXIST;
>+                OFN_HIDEREADONLY | OFN_PATHMUSTEXIST | OFN_ENABLESIZING | 
>+                OFN_EXPLORER | OFN_ENABLEHOOK;
>+    ofn.lpfnHook = FilePickerHook;
> 
>     // Handle add to recent docs settings
>     nsCOMPtr<nsIPrivateBrowsingService> pbs =
>       do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID);
>     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.Flags |= OFN_FILEMUSTEXIST | OFN_ALLOWMULTISELECT | 
>-                     OFN_EXPLORER | OFN_ENABLEHOOK;
>+        ofn.Flags |= OFN_FILEMUSTEXIST | OFN_ALLOWMULTISELECT;
> 
>         // 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
(Assignee)

Comment 19

6 years ago
Created attachment 555420 [details] [diff] [review]
Patch for file picker popups are shown in last visible position, but should not if those positions are now non visible. v3.

Typo in comment.
Attachment #555416 - Attachment is obsolete: true
Attachment #555420 - Flags: review+
(Assignee)

Comment 20

6 years ago
Pushed to try, results:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=ec8056cbd584
(Assignee)

Comment 21

6 years ago
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/59c205c5d0ad
http://hg.mozilla.org/mozilla-central/rev/59c205c5d0ad
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9

Updated

6 years ago
Depends on: 684506
You need to log in before you can comment on or make changes to this bug.