Closed Bug 977957 Opened 10 years ago Closed 10 years ago

Australis: Browser hangs when alert() comes up while dragging a toolbarbutton

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

29 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox29 + wontfix
firefox30 + fixed
firefox31 --- verified

People

(Reporter: alice0775, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P3][workaround in comment 29][fixed from bug 100180] [testday-20140530])

Attachments

(2 files)

Attached file sample
Steps To Reproduce
1. Open sample html and click "click to alert 10 seconds later"
2. Enter Customize mode
3. Dragging a toolbutton (until alert() pops up)

Actual Results:
Browser hangs.

Prior to Australis, it recovers by pressing ESC keys.
However, Australis is completely hang, no way to recover without killing a process.
Summary: Browser hangs when alert() comes up while dragging a toolbutton → Australis: Browser hangs when alert() comes up while dragging a toolbutton
Stack when browser hang: bp-14102be4-2bd6-4319-bfc0-3e8232140228
Component: Toolbars and Customization → Event Handling
Product: Firefox → Core
Version: 30 Branch → 29 Branch
I cannot reprodudce on Ubuntu.
So, this maybe Windows only bug.
Component: Event Handling → Widget: Win32
Setting prompts.tab_modal.enabled = false, it recovers by pressing ESC keys.
Blocks: 59314
Possibly related to bug 626963 and bug 846960.
Summary: Australis: Browser hangs when alert() comes up while dragging a toolbutton → Australis: Browser hangs when alert() comes up while dragging a toolbarbutton
Whiteboard: [Australis:P2]
Yeah, I can't repro on Mac either... although I do wonder if there's some way we could kill the drag when leaving customize mode. I've experimented with doing that but I haven't managed to make it do something tangible on mac. I wonder what's causing the hang - the stack isn't particularly informative.
Bad, but seems very low probability. I'd expect that other things doing drag-n-drop (like the old customize mode!) probably have similar issues.
Whiteboard: [Australis:P2] → [Australis:P3]
(In reply to Justin Dolske [:Dolske] from comment #6)
> Bad, but seems very low probability. I'd expect that other things doing
> drag-n-drop (like the old customize mode!) probably have similar issues.

No, no. not similar.

I mentionned in comment #0,  the UI lock is revovers by pressinbg ESC in the old customize mode.
However, pressinbg ESC does not help any more in the new customize mode.

This hang has no way to recover.
The UI isn't actually hung on Windows, either - the mouse cursor is blocked because it's still dragging, but keyboard shortcuts work, and if you ctrl-tab back to the customize tab, you can end the drag and recover from there. That's not obvious, and we should obviously figure out a way to fix this regardless, but that's what seems to be the issue.
Severity: critical → major
Enn graciously said he could take a look at this. :-)
Assignee: nobody → enndeakin
Component: Widget: Win32 → Drag and Drop
This is pretty much the same as 100180. There's a nested event loop when the prompt opens.
Should we P-/wontfix/dupe this, then? Nested event loops are a basic browser problem, so this wouldn't appear to be new or special. (Such loops were a big risk of tab-modal prompts, but never actually became a common issue.)
(In reply to Justin Dolske [:Dolske] from comment #11)
> Should we P-/wontfix/dupe this, then? Nested event loops are a basic browser
> problem, so this wouldn't appear to be new or special. (Such loops were a
> big risk of tab-modal prompts, but never actually became a common issue.)

Mm... Neil, ISTR we discussed this on our way back from lunch in Toronto, and you said there might be a workaround, but I don't recall what that was... I also seem to recall you at one point told me you *can* cancel drags on Windows (which, from looking at bug 100180 and this, is the most seriously affected platform)... Could you provide more info? I might be able to look at this the coming week if I have sufficient info on how to actually cancel the drag - I learned a little bit about the tab-modal dialog code in the onbeforeunload bugs I've been working on.
Flags: needinfo?(enndeakin)
> Mm... Neil, ISTR we discussed this on our way back from lunch in Toronto,
> and you said there might be a workaround, but I don't recall what that
> was...

I think I suggested to not show dialogs while one was dragging on Windows and Mac.

> I also seem to recall you at one point told me you *can* cancel drags
> on Windows (which, from looking at bug 100180 and this, is the most
> seriously affected platform)... Could you provide more info?

You need to change nsNativeDragSource::QueryContinueDrag to return DRAGDROP_S_CANCEL in the right circumstances. That isn't the right fix though.
Flags: needinfo?(enndeakin)
Any chance to see that bug fixed for 29? Thanks
Neil, any progress on this bug? Thanks :)
Flags: needinfo?(enndeakin)
I'm guessing that th right fix involves handling dragging during nested event loops, something I'm unlikely to be able to fix.
Flags: needinfo?(enndeakin)
What about some wrong fixes? Workarounds landable on 29 at this stage?

If it's impossible to get anything fixed here for 29, then let's be explicit about it, rather than letting the bug sit.
Flags: needinfo?(enndeakin)
The wrong fixes are listed in comment 13.
Assignee: enndeakin → nobody
Flags: needinfo?(enndeakin)
Naive question time - can we not have CustomizeMode.jsm listen for DOMWillOpenModalDialog, and programmatically cancel the drag session during the capturing phase?

I'm going to see if that's even possible.
And it's become clear now that we don't expose a thing that allows us to cancel the drag.

I have another idea - one sec, lemme posit a patch...
Attached patch ExperimentSplinter Review
So I'm just putting this out there - I have no idea if this is a good idea or not.

I've noticed that if the modal dialog is _window modal_ as opposed to _tab modal_, pressing Esc lets the user get back to business without a full restart.

So this patch makes it so that if a drag session is currently active, we prefer window modal dialogs.

How do we feel about that?
Attachment #8404312 - Flags: feedback?(enndeakin)
Attachment #8404312 - Flags: feedback?(dolske)
Pretty sure this is a trap, but I'll take responsibility for this one.
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Mike, is it a trap for 29 ? :)
(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Mike, is it a trap for 29 ? :)

Maybe - let's see what Enn, Dolske or others say about my suggestion.
I think it's worth trying it if it improves things.
(In reply to Mike Conley (:mconley) from comment #21)

> I've noticed that if the modal dialog is _window modal_ as opposed to _tab
> modal_, pressing Esc lets the user get back to business without a full
> restart.

Huh. Do you know why that's the case? I'd would have guessed that window prompts were the problem, not tab-modal. Makes me wonder if we're not doing something for tab-modal prompts that we should be doing...

I'm kind of wary about this patch, because it effectively allows content to force a window-modal prompt. As noted in comment 6, I think this bug's impact is bad but it's very unlikely to happen. But we know from other recent bugs that malicious sites do abuse prompts, and that tab-modal prompts are an effective mitigation to that. So I'd be concerned that this patch opens a hole via tricking the user into dragging something on the page (simple), and then launching some form of infinite/confusing prompt abuse.

In fact, given the rarity, I'd be fine with simply shipping without a fix for this bug.

Sorta feels like the right long-term fix will be to find/implement a way to cancel the drag when a prompt is shown (or, more generally, when the selected tab changes). That seems like the right behavior anyway?
Attachment #8404312 - Flags: feedback?(enndeakin)
Attachment #8404312 - Flags: feedback?(dolske)
Attachment #8404312 - Flags: feedback-
(In reply to Justin Dolske [:Dolske] from comment #26)
> Sorta feels like the right long-term fix will be to find/implement a way to
> cancel the drag when a prompt is shown (or, more generally, when the
> selected tab changes). That seems like the right behavior anyway?

No, the right long term fix is to fix bug 100180. You can't cancel drags on Mac anyway.
If bug 100180 is too long term, finding a way for tab-modal prompts to also be "escapable" like the window modal ones might be an acceptable solution if it's been acceptable for window modals since *glances at bug 100180*... 2001.

At any rate, based on dolske's comment 26, I'm tempted to WONTFIX this for 29, and try to find a solution down the line.

Marking as WONTFIX for 29. Please change it back if there's any disagreement.
Depends on: 100180
Another workaround for this one:

Because keyboard input is not suspended, the user can Tab focus to the "OK" of the modal dialog, and then use the spacebar to dismiss it.
Whiteboard: [Australis:P3] → [Australis:P3][workaround in comment 29]
Based on conversations with TimAbraldes and markh in bug 100180, I'm starting to suspect that fixing 100180 might be a little beyond me - at least for the short term.

So I think I want to change tack a little bit - instead of attempting to fix 100180, I'm going to see if I can make it so that the tab-modal dialogs allow us to kill the drag session with the Esc key, like the window modal dialogs. At least in that case, we'll be not worse than we've already been.

Once that's done, we can think about how to fix bug 100180.
Bug 100180 landed, so this should be fixed now.

I'll let it bake on m-c until tomorrow, and then I'll request Aurora uplift approval.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Whiteboard: [Australis:P3][workaround in comment 29] → [Australis:P3][workaround in comment 29][fixed from bug 100180]
The patch in bug 100180 was uplifted to Aurora.
QA Whiteboard: [good first verify]
Verified fixed using Windows 7 64 bit (es-AR) and Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0. Verified in latest Nightly as well.
Status: RESOLVED → VERIFIED
Whiteboard: [Australis:P3][workaround in comment 29][fixed from bug 100180] → [Australis:P3][workaround in comment 29][fixed from bug 100180] [testday-20140530]
You need to log in before you can comment on or make changes to this bug.