Closed
Bug 1260850
Opened 8 years ago
Closed 8 years ago
Open File Dialog Box Stuck
Categories
(Firefox :: File Handling, defect, P1)
Tracking
()
RESOLVED
FIXED
Firefox 51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: dhtmlkitchen, Assigned: spohl)
References
(Blocks 1 open bug)
Details
(Whiteboard: tpi:+)
Attachments
(2 files, 2 obsolete files)
(deleted),
application/x-shockwave-flash
|
Details | |
(deleted),
patch
|
spohl
:
review+
|
Details | Diff | Splinter Review |
1. Click "Browse" 2. Look to select a file, 3. Switch to Finder 4. Switch back to FF Result: File dialog is stuck open. Cancel doesn't work, Open doesn't work. I have to quit the browser. Step 3 might or might not be necessary. Screen recording: https://www.youtube.com/edit?video_id=J6tP_b9GcdI&video_referrer=watch
Reporter | ||
Comment 1•8 years ago
|
||
I closed the window from where the browse button originated. It's a private website (artistworks) that requires login. Also notice the scren menubar.
Comment 2•8 years ago
|
||
Can you reproduce this reliably? As in, if you use the browse button again on the same website, switch to finder and back, does the same thing happen again?
Flags: needinfo?(dhtmlkitchen)
Comment 3•8 years ago
|
||
FWIW I couldn't reproduce on Mac OS X using Nightly. I assume yours is the release version Firefox 45. The video clearly shows broken behavior, but it can't be used in an "attack" so we don't need to keep it hidden.
Group: firefox-core-security
Comment 5•8 years ago
|
||
Hi Garrett Smith, I'm moving your issue to Firefox - File Handling component. Activity Streams is an add-on and has nothing to do with your issue. Thanks.
Component: Activity Streams: General → File Handling
Reporter | ||
Comment 6•8 years ago
|
||
Version: 45 Branch Leak a file dialog box and force the user to force-quit the browser. — no security issue here? I cannot describe any attack, but this sounds pretty bad to me. Thank you.
Flags: needinfo?(dhtmlkitchen)
Reporter | ||
Comment 7•8 years ago
|
||
I will give steps to reproduce this when I can. The issue described and shown in the video has happened to me twice to me.
Comment 8•8 years ago
|
||
spohl, this sounds similar to an issue we discussed recently. Any clues?
Flags: needinfo?(spohl.mozilla.bugs)
Assignee | ||
Comment 9•8 years ago
|
||
This does sound related to other issues we've been seeing. However, I've been unable to reproduce this. I've been using http://www.cs.tut.fi/~jkorpela/forms/file.html among others to try and reproduce but have had no success with release or nightly. Garrett, are you able to reproduce this issue on http://www.cs.tut.fi/~jkorpela/forms/file.html? If not, is there another public site that reproduces this for you?
Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(dhtmlkitchen)
Reporter | ||
Comment 10•8 years ago
|
||
I just accidentally reproduced this using Gmail non-basic verion attach file. I tried to reproduce it again but was unable to. Using Firefox 45.0.2 I had to use activity monitor to force-quit. The user experience is blocked and hijacked. The user is prevented from any further action, and is not able to quit the browser. To quit, the user must launch Activity Monitor. A lot of users won't know how to do that, and will feel helpless and stuck. If this problem can be triggered and reproduced, even if only some of the time, the triggering page can be used as an attack to demand that the user perform some action such as calling a phone number to provide credit card details, etc. I don't know how to consistently reproduce this problem.
Keywords: steps-wanted
Comment 11•8 years ago
|
||
I'm not sure if the dialog priority causes this issue. It seemed to me that I run into a similar situation. When background popped up a alert, the Open dialog was hang there. So, I suggest reporter to check if any alert dialog causes this issue. Thanks. Have a nice day! Attach the demo video (OPEN_FILE_DIALOG_STUCK-SITUATION02.swf) * You can open it by using Firefox.
Comment 12•8 years ago
|
||
Reporter | ||
Comment 13•8 years ago
|
||
I can't open that SWF. I don't have flash on my system. I did not see any alert or other dialog boxes besides the Open File box. By "check" did you mean look? If not, please detail. Thank you,
Flags: needinfo?(dhtmlkitchen)
Comment 14•8 years ago
|
||
Trivial STR: - open browser console. Make sure you can evaluate script in it (open web console, settings, tick 'enable browser and add-on debugging toolboxes'). - run: setTimeout(() => Services.prompt.alert(window, "Hi", null), 5000); - switch back to the main window and hit Cmd-O (or use one of the testcases that has a browse / <input type=file> button) - wait for the setTimeout to hit. So basically, if any other thing runs an event on the main thread that causes us to pop up a sheet-alert, while the file opening dialog is open, stuff breaks. :spohl, does this help you investigate further?
Flags: needinfo?(spohl.mozilla.bugs)
Comment 15•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #14) > Trivial STR: Just to be clear: these are trivial in the sense that they are contrived, but simple to follow. I expect that "in the wild", like in the swf screencast William attached, we show a slow script dialog or some other dialog for unrelated reasons, and inadvertently make the browser hard to interact with for the user in question.
Comment 16•8 years ago
|
||
Thanks for Gijs's help!:) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Hi, Stephen, Good day. I was wondering if you have any comment on this bug? Thank you!
Flags: needinfo?(spohl.mozilla.bugs)
Updated•8 years ago
|
Summary: Open File Dialon Box Stuck → Open File Dialog Box Stuck
Assignee | ||
Comment 17•8 years ago
|
||
Can this be reproduced with latest nightly? Even with the steps in comment 14 I can't reproduce the problem with a local build off of current trunk.
Flags: needinfo?(whsu)
Flags: needinfo?(spohl.mozilla.bugs)
Comment 18•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #17) > Can this be reproduced with latest nightly? Even with the steps in comment > 14 I can't reproduce the problem with a local build off of current trunk. I just tried with nightly from yesterday, on OSX 10.11, and I can reproduce using the cmd-O option to open the filepicker. I haven't tried the input type=file case myself, but I would expect it to work the same way. You can still move the filepicker out of the way, accept the modal dialog, and then the filepicker can be interacted with again - but the filepicker typically appears (and stays) on top of the modal dialog, so that is not at all obvious. Does that help?
Flags: needinfo?(spohl.mozilla.bugs)
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to :Gijs Kruitbosch (PTO recovery mode) from comment #18) > (In reply to Stephen A Pohl [:spohl] from comment #17) > > Can this be reproduced with latest nightly? Even with the steps in comment > > 14 I can't reproduce the problem with a local build off of current trunk. > > I just tried with nightly from yesterday, on OSX 10.11, and I can reproduce > using the cmd-O option to open the filepicker. I haven't tried the input > type=file case myself, but I would expect it to work the same way. You can > still move the filepicker out of the way, accept the modal dialog, and then > the filepicker can be interacted with again - but the filepicker typically > appears (and stays) on top of the modal dialog, so that is not at all > obvious. > > Does that help? Ah, yes, I thought we were talking about completely stuck and that there was no way to dismiss the modal dialog. Thanks! Will have another look tomorrow. Keeping n-i set.
Flags: needinfo?(whsu)
Assignee | ||
Comment 20•8 years ago
|
||
Ok, using mozregression it looks like this bug has been around forever. However, there is a point in time when things have improved. We used to completely hang the browser where neither the modal dialog nor the open file dialog could be dismissed. This changed and we can now switch back to the browser, dismiss the modal dialog and then dismiss the open file dialog. This occurred here: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=20ca234fd62b&tochange=4b6d63b05a0a And my money is on bug 996848 as the reason why things have improved. To make sure we have the same expectations for a fix: should the fix be to not steal away focus from the open file dialog, allowing it to be dismissed? Or should it lose focus, but continue to be focusable and dismissable once regaining focus?
Flags: needinfo?(spohl.mozilla.bugs)
Comment 21•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #20) > To make sure we have the same expectations for a fix: should the fix be to > not steal away focus from the open file dialog, allowing it to be dismissed? > Or should it lose focus, but continue to be focusable and dismissable once > regaining focus? I'm not a UX person, so I'm not the best person to answer this question. On Windows, these dialogs are fully modal, so it is impossible for another modal to come up while the filepicker is open. Testing Chrome on OS X, the alert comes up as a non-sheet modal dialog *on top of* the (sheet'd) filepicker, so you can see it and it is clear that that modal is preventing you from interacting with the filepicker until you accept the modal. I think that might be the most obvious interaction here, but I don't know if that's hard to implement. Out of the two options you describe I guess I would prefer the former, although it would also be nice that if it did end up losing focus it would continue to be focusable and dismissable once you focused it again. Really, any of these three options sound better than the current situation (if to different degrees). Does that help? You could ask UX people like :bram or :shorlander for advice if you think that would be more helpful than my ramblings. :-)
Flags: needinfo?(spohl.mozilla.bugs)
Assignee | ||
Comment 22•8 years ago
|
||
Modal dialogs have a long history (read: many, many bugs to read through) in our code base on OSX and I'm just starting to wrap my head around it. But it appears that when we're app modal (file picker), we can still have a nested loop that tries to display a window modal dialog (alert dialog). I couldn't find an existing way for nsWindowWatcher::OpenWindowInternal to know whether we're app modal, and my patch here is just a suggestion. Calling SetFakeModal when we're app modal still steals the focus away from the file picker, but the file picker remains app modal and only the file picker can be interacted with until it is dismissed. Aside from the initial loss of focus, things seem to be working as expected in my testing. Markus, is this a direction worth pursuing?
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8784382 -
Flags: feedback?(mstange)
Updated•8 years ago
|
Priority: -- → P1
Whiteboard: tpi:+
Comment 23•8 years ago
|
||
FWIW, this patch fixes the issue I was hitting in bug 1297499. I personally run into this bug at least twice a week, since I upload large files to Air Mozilla and YouTube for my livehacking.
Comment 25•8 years ago
|
||
Comment on attachment 8784382 [details] [diff] [review] WIP Review of attachment 8784382 [details] [diff] [review]: ----------------------------------------------------------------- Modal dialogs are a part of widget/cocoa that I have been lucky enough to avoid so far, so I don't really know much about them. Since this patch helps, I think it's worth pursuing. I think it's possible to come up with a better API than the one in this patch though. For example, overloading nsresult for a boolean return value isn't that great. And having a platform-specific workaround in cross-platform code isn't great either. As a suggestion, could you instead add a method like nsIWidget::OnOpeningModalChild, which does "if (isrunningappmodel) { this->setfakemodal(true); }" or something like that?
Attachment #8784382 -
Flags: feedback?(mstange) → feedback+
Assignee | ||
Comment 26•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #25) > Comment on attachment 8784382 [details] [diff] [review] > I think it's possible to come up with a better API than the one in this > patch though. For example, overloading nsresult for a boolean return value > isn't that great. Yes, definitely. That was not meant to make it into a final patch. Changed to bool. > And having a platform-specific workaround in > cross-platform code isn't great either. As a suggestion, could you instead > add a method like nsIWidget::OnOpeningModalChild, which does "if > (isrunningappmodel) { this->setfakemodal(true); }" or something like that? I couldn't quite figure out how to make this work. However, nsIWidget already seems to have platform specific methods, such as SetFakeModal and RoundsWidgetCoordinatesTo that get overridden by Cocoa's widget/cocoa/nsWidgetCocoa.[h|mm]. In light of this, is this patch acceptable? Or would you like me to go back and perform more surgery?
Attachment #8784382 -
Attachment is obsolete: true
Attachment #8788913 -
Flags: review?(mstange)
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8788913 [details] [diff] [review] Patch Review of attachment 8788913 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/nsIWidget.h @@ +652,5 @@ > SetModal(aModal); > } > > /** > + * Are we app modal on Cocoa. Maybe I should change this comment to be more cross-platform, something like: > Are we app modal. Currently only implemented on Cocoa.
Comment 28•8 years ago
|
||
Comment on attachment 8788913 [details] [diff] [review] Patch Review of attachment 8788913 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/nsIWidget.h @@ +652,5 @@ > SetModal(aModal); > } > > /** > + * Are we app modal on Cocoa. Yes, the proposed comment sounds better.
Attachment #8788913 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 29•8 years ago
|
||
Thanks, Markus! Changed comment, carrying over r+.
Attachment #8788913 -
Attachment is obsolete: true
Attachment #8789485 -
Flags: review+
Assignee | ||
Comment 30•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce044fe01c59
Assignee | ||
Comment 31•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ceeb69dfe4a5b7347a7d74a50f6fb5514ad2b1ba Bug 1260850: Ensure that FilePicker (and other native app modal dialogs) on OSX remains responsive when a window modal dialog opens in the background. r=mstange
Assignee | ||
Updated•8 years ago
|
Keywords: steps-wanted
Comment 32•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ceeb69dfe4a5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Comment 33•8 years ago
|
||
Do we know if this bug is on 51 only? Is this something we should uplift? (Dunno how safe that is)
Flags: needinfo?(spohl.mozilla.bugs)
Assignee | ||
Comment 34•8 years ago
|
||
Per comment 20, this is not a regression and behavior has actually slightly improved over time. Personally, I would prefer letting this change ride the trains and give us as much time as possible to catch any possible fallout.
Flags: needinfo?(spohl.mozilla.bugs)
You need to log in
before you can comment on or make changes to this bug.
Description
•