840855 - Defect - Dialogs intermittently fail to accept touch input

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Input, defect, P2)

Description

[Asa Dotzler [:asa]]

Modal message dialogs, like those triggered by a website using onbeforeunload or by Metro Firefox features like Sync setup or Import, all seem broken.

They're modal but they don't seem to capture the tap which falls through the dialog to the page behind it. This means that you cannot dismiss the dialog. This hangs Firefox.

Comment 1

[Matt Brubeck (:mbrubeck)]

I've seen this before, but I can't reproduce it at the moment... what build and device are you seeing this in, and can you post exact steps to reproduce?  Can you still reproduce it in the latest nightly?  (I wonder if it depends on which web content is behind the dialog; could be related to bug 835175.)

Comment 2

[Asa Dotzler [:asa]]

I can reproduce in today's nightly build by opening Firefox to Firefox Start, swiping in the charms bar, tapping the Settings charm, then Options, and tapping the Clear Private Data item.  I see the same thing with Sync setup and with the onbeforeunload handler at my movable type blog.

Comment 3

[Matt Brubeck (:mbrubeck)]

I can't reproduce this 100%, but it happens much more with touch input than with mouse input.

I'm guessing that spinning the event loop like this might be a bad idea:
http://hg.mozilla.org/mozilla-central/file/081cf5b0121e/browser/metro/base/content/bindings/dialog.xml#l101

Comment 6

[Matt Brubeck (:mbrubeck)]

There appear to be multiple causes for this.  The patch in bug 840643 fixes this for various Firefox Sync dialogs, but I'm still seeing it intermittently for other dialogs.

Comment 7

[Brian R. Bondy [:bbondy]]

I'm not sure how to reproduce this at all, is there any page I can use that consistently reproduces?

Comment 8

[Asa Dotzler [:asa]]

I used to be able to pretty reliably reproduce this with the Clear Private Data button in Options but that button seems completely broken now. 

I can still reliably reproduce at sites using onbeforeunload. For a test, go to http://jsfiddle.net/zalun/yFuPr/ and tap the Run item in the page header to trigger the Message Dialog. Then, using touch (it fails more in touch than in mouse click) tap either of the buttons and see them do nothing. Also, when it's in this state, I can create new tabs and navigate all around using the keyboard and the Message Dialog stays up, even if I visit another site.

Comment 9

[Asa Dotzler [:asa]]

Updating summary to reflect that is is more of a problem with touch than pointer.

Comment 10

[Brian R. Bondy [:bbondy]]

Awesome I can reproduce with touch on that page, thanks.

Comment 11

[Jim Mathies [:jimm]]

Another good example - in a bugzilla bug page, hit the top search button without entering any terms or bug numbers.

Comment 12

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

I believe this should be fixed when the patches in bug 866304 land.  Assigning myself to test that that is the case.

Comment 14

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Unassigning myself so this leaves my bugzilla dashboard. I'll pick this back up as part of bug 892575.

Comment 16

[Jim Mathies [:jimm]]

for some discovery work, this seemed to regress pretty bad when we turned on apz.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

I can reproduce this problem using the jsfiddle in comment 8 but I'm not convinced this is APZC-related. For one thing if I alt-tab away and then alt-tab back, Firefox is killed and restarted. This seems to indicate to me that the UI thread is getting locked up somewhere and Windows is killing it. The UI thread getting locked up would also explain why the dialog is nonresponsive. I tried attaching in Visual Studio but Windows keeps killing the process so it's hard for me to get a handle on what's going on. I'm going to leave this blocking bug 886321 since it seems like a more general meta bug but I'm going to take it off my radar unless somebody can show me that it's APZC-related.

Comment 18

[Jim Mathies [:jimm]]

Placing a break point on widget's DispatchEvent shows no events being processed. This is with the onbeforeunload jsfiddle sample. We may have different issues with other dialogs.

Comment 19

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

FWIW, I've been testing this on Aurora and haven't been able to reproduce. Could this be a debug-build-only bug? Or perhaps an issue only on m-c?

Comment 20

[Jim Mathies [:jimm]]

We get caught up in a modal loop while dispatching all gecko events from down in app shell. This might be tricky to fix.

>	xul.dll!MetroAppShell::ProcessNextNativeEvent(bool mayWait=false) Line 307	C++
 	xul.dll!nsBaseAppShell::DoProcessNextNativeEvent(bool mayWait=false, unsigned int recursionDepth=1) Line 142	C++
 	xul.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00000014, bool mayWait=true, unsigned int recursionDepth=1) Line 278	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x02b5c290) Line 598	C++
 	xul.dll!NS_InvokeByIndex(nsISupports * that=0x027635f8, unsigned int methodIndex=8, unsigned int paramCount=2, nsXPTCVariant * params=0x02b5c280) Line 71	C++
 	xul.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD) Line 2111	C++
 	xul.dll!XPC_WN_CallMethod(JSContext * cx=0x0485d6e8, unsigned int argc=1, JS::Value * vp=0x02b5c424) Line 1308	C++
 	2ea629ac()	Unknown
 	[Frames below may be incorrect and/or missing]	
 	170d8cb0()	Unknown
 	02e10991()	Unknown
 	mozjs.dll!EnterBaseline(JSContext * cx=0x0485d6e8, js::jit::EnterJitData & data={...}) Line 123	C++
 	mozjs.dll!js::jit::EnterBaselineAtBranch(JSContext * cx=0x0485d6e8, js::StackFrame * fp=0x02b5ca54, unsigned char * pc=0x04a13e9e) Line 206	C++
 	mozjs.dll!Interpret(JSContext * cx=0x02b5ca54, js::RunState & state={...}) Line 1527	C++
 	mozjs.dll!js::RunScript(JSContext * cx=0x0485d6e8, js::RunState & state={...}) Line 420	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT) Line 482	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, const JS::Value & thisv={...}, const JS::Value & fval={...}, unsigned int argc=8, JS::Value * argv=0x02b5cef0, JS::MutableHandle<JS::Value> rval={...}) Line 513	C++
 	mozjs.dll!JS_CallFunctionValue(JSContext * cx=0x0485d6e8, JSObject * objArg=0x0c44a7f0, JS::Value fval={...}, unsigned int argc=8, JS::Value * argv=0x02b5cef0, JS::Value * rval=0x02b5cddc) Line 5178	C++
 	xul.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper=0x0f56f5e0, unsigned short methodIndex=7, const XPTMethodDescriptor * info_=0x02811ae0, nsXPTCMiniVariant * nativeParams=0x02b5cfa0) Line 1424	C++
 	xul.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex=7, const XPTMethodDescriptor * info=0x02811ae0, nsXPTCMiniVariant * params=0x02b5cfa0) Line 591	C++
 	xul.dll!PrepareAndDispatch(nsXPTCStubBase * self=0x12dd9ad0, unsigned int methodIndex=7, unsigned int * args=0x02b5d048, unsigned int * stackBytesToPop=0x02b5d038) Line 85	C++
 	xul.dll!SharedStub() Line 113	C++
 	xul.dll!nsDocumentViewer::PermitUnload(bool aCallerClosesWindow=123, bool * aPermitUnload=0x09688d30) Line 1163	C++
 	nss3.dll!_MD_CURRENT_THREAD() Line 314	C
 	xul.dll!nsDocumentViewer::PermitUnload(bool aCallerClosesWindow=false, bool * aPermitUnload=0x02b5d29f) Line 1188	C++
 	xul.dll!nsDocShell::InternalLoad(nsIURI * aURI=0x0f885650, nsIURI * aReferrer=0x0f885708, nsISupports * aOwner=0x00000000, unsigned int aFlags=0, const wchar_t * aWindowTarget=0x00000000, const char * aTypeHint=0x72d6aef8, const nsAString_internal & aFileName={...}, nsIInputStream * aPostData=0x00000000, nsIInputStream * aHeadersData=0x00000000, unsigned int aLoadType=4, nsISHEntry * aSHEntry=0x0f2be0d0, bool aFirstParty=true, const nsAString_internal & aSrcdoc={...}, nsIDocShell * * aDocShell=0x00000000, nsIRequest * * aRequest=0x00000000) Line 9339	C++
 	xul.dll!nsDocShell::LoadHistoryEntry(nsISHEntry * aEntry=0x0f2be0d0, unsigned int aLoadType=4) Line 11050	C++
 	xul.dll!nsDocShell::LoadURI(nsIURI * aURI=0x0f885650, nsIDocShellLoadInfo * aLoadInfo=0x1715e1c0, unsigned int aLoadFlags=0, bool aFirstParty=false) Line 1401	C++
 	xul.dll!nsSHistory::InitiateLoad(nsISHEntry * aFrameEntry=0x0f885650, nsIDocShell * aFrameDS=0x1715e1c0, long aLoadType=2) Line 1733	C++
 	xul.dll!nsSHistory::LoadEntry(int aIndex=11, long aLoadType=2, unsigned int aHistCmd=3) Line 1601	C++
 	xul.dll!nsSHistory::GoBack() Line 826	C++
 	xul.dll!nsDocShell::GoBack() Line 4047	C++
 	xul.dll!NS_InvokeByIndex(nsISupports * that=0x096b3bdc, unsigned int methodIndex=5, unsigned int paramCount=0, nsXPTCVariant * params=0x02b5d970) Line 71	C++
 	xul.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD) Line 2111	C++
 	xul.dll!XPC_WN_CallMethod(JSContext * cx=0x0485d6e8, unsigned int argc=0, JS::Value * vp=0x03c4ee00) Line 1308	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT) Line 456	C++
 	mozjs.dll!Interpret(JSContext * cx=0x03c4ee10, js::RunState & state={...}) Line 2466	C++
 	mozjs.dll!js::RunScript(JSContext * cx=0x0485d6e8, js::RunState & state={...}) Line 420	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT) Line 482	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, const JS::Value & thisv={...}, const JS::Value & fval={...}, unsigned int argc=1, JS::Value * argv=0x02b5e598, JS::MutableHandle<JS::Value> rval={...}) Line 513	C++
 	mozjs.dll!JS_CallFunctionValue(JSContext * cx=0x0485d6e8, JSObject * objArg=0x054eb340, JS::Value fval={...}, unsigned int argc=1, JS::Value * argv=0x02b5e598, JS::Value * rval=0x02b5e484) Line 5178	C++
 	xul.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper=0x09997428, unsigned short methodIndex=5, const XPTMethodDescriptor * info_=0x03004c98, nsXPTCMiniVariant * nativeParams=0x02b5e648) Line 1424	C++
 	xul.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex=5, const XPTMethodDescriptor * info=0x03004c98, nsXPTCMiniVariant * params=0x02b5e648) Line 591	C++
 	xul.dll!PrepareAndDispatch(nsXPTCStubBase * self=0x09bc3780, unsigned int methodIndex=5, unsigned int * args=0x02b5e6f0, unsigned int * stackBytesToPop=0x02b5e6e0) Line 85	C++
 	xul.dll!SharedStub() Line 113	C++
 	xul.dll!NS_InvokeByIndex(nsISupports * that=0x09bc3780, unsigned int methodIndex=5, unsigned int paramCount=1, nsXPTCVariant * params=0x02b5e7a0) Line 71	C++
 	xul.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD) Line 2111	C++
 	xul.dll!XPC_WN_CallMethod(JSContext * cx=0x0485d6e8, unsigned int argc=1, JS::Value * vp=0x03c4ed08) Line 1308	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT) Line 456	C++
 	mozjs.dll!Interpret(JSContext * cx=0x03c4ed18, js::RunState & state={...}) Line 2466	C++
 	mozjs.dll!js::RunScript(JSContext * cx=0x0485d6e8, js::RunState & state={...}) Line 420	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT) Line 482	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x0485d6e8, const JS::Value & thisv={...}, const JS::Value & fval={...}, unsigned int argc=1, JS::Value * argv=0x02b5f290, JS::MutableHandle<JS::Value> rval={...}) Line 513	C++
 	mozjs.dll!JS_CallFunctionValue(JSContext * cx=0x0485d6e8, JSObject * objArg=0x091d1850, JS::Value fval={...}, unsigned int argc=1, JS::Value * argv=0x02b5f290, JS::Value * rval=0x02b5f268) Line 5178	C++
 	xul.dll!mozilla::dom::EventListener::HandleEvent(JSContext * cx=0x0485d6e8, JS::Handle<JSObject *> aThisObj={...}, nsDOMEvent & event={...}, mozilla::ErrorResult & aRv={...}) Line 45	C++
 	xul.dll!mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget *>(mozilla::dom::EventTarget * const & thisObj=0x04d01288, nsDOMEvent & event={...}, mozilla::ErrorResult & aRv={...}, mozilla::dom::CallbackObject::ExceptionHandling aExceptionHandling=eReportExceptions) Line 51	C++
 	xul.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct=0x09827ea0, const mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener,nsIDOMEventListener> & aListener={...}, nsIDOMEvent * aDOMEvent=0x130080a8, mozilla::dom::EventTarget * aCurrentTarget=0x04d01288, nsCxPusher * aPusher=0x02b5f504) Line 953	C++
 	xul.dll!nsEventListenerManager::HandleEventInternal(nsPresContext * aPresContext=0x04ace8e8, mozilla::WidgetEvent * aEvent=0x02b5f501, nsIDOMEvent * * aDOMEvent=0x04d01288, mozilla::dom::EventTarget * aCurrentTarget=0x04d01288, nsEventStatus * aEventStatus=0x02b5f4f8, nsCxPusher * aPusher=0x02b5f504) Line 1030	C++
 	xul.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x04ace8e8, mozilla::WidgetEvent * aEvent=0x02b5f584, nsIDOMEvent * * aDOMEvent=0x02b5f4f4, mozilla::dom::EventTarget * aCurrentTarget=0x04d01288, nsEventStatus * aEventStatus=0x02b5f4f8, nsCxPusher * aPusher=0x02b5f504) Line 326	C++
 	xul.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, ELMCreationDetector & aCd={...}, nsCxPusher * aPusher=0x02b5f504) Line 198	C++
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem> & aChain={...}, nsEventChainPostVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x02b5f640, ELMCreationDetector & aCd={...}, nsCxPusher * aPusher=0x02b5f504) Line 295	C++
 	xul.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x04d01288, nsPresContext * aPresContext=0x04ace8e8, mozilla::WidgetEvent * aEvent=0x02b5f584, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x02b5f570, nsDispatchingCallback * aCallback=0x02b5f640, nsCOMArray<mozilla::dom::EventTarget> * aTargets=0x00000000) Line 612	C++
 	xul.dll!PresShell::DispatchTouchEvent(mozilla::WidgetEvent * aEvent=0x0ee36960, nsEventStatus * aStatus=0x02b5f704, nsPresShellEventCB * aEventCB=0x02b5f640, bool aTouchIsNew=false) Line 6994	C++
 	xul.dll!PresShell::HandleEventInternal(mozilla::WidgetEvent * aEvent=0x00000000, nsEventStatus * aStatus=0x02b5f704) Line 6881	C++
 	xul.dll!PresShell::HandlePositionedEvent(nsIFrame * aTargetFrame=0x00000000, mozilla::WidgetGUIEvent * aEvent=0x0ee36960, nsEventStatus * aEventStatus=0x02b5f704) Line 6602	C++
 	xul.dll!PresShell::HandleEvent(nsIFrame * aFrame=0x00000000, mozilla::WidgetGUIEvent * aEvent=0x0ee36900, bool aDontRetargetEvents=232, nsEventStatus * aEventStatus=0x02b5f704) Line 6404	C++
 	xul.dll!nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent * aEvent=0x0ee36901, nsView * aView=0x04a68fe0, nsEventStatus * aStatus=0x02b5f704) Line 744	C++
 	xul.dll!nsView::HandleEvent(mozilla::WidgetGUIEvent * aEvent=0x0ee36960, bool aUseAttachedEvents=true) Line 1085	C++
 	xul.dll!MetroWidget::DispatchEvent(mozilla::WidgetGUIEvent * event=0x0ee36960, nsEventStatus & aStatus=nsEventStatus_eIgnore) Line 1245	C++
 	xul.dll!mozilla::widget::winrt::MetroInput::DeliverNextQueuedTouchEvent() Line 1241	C++
 	xul.dll!nsRunnableMethodImpl<void (__thiscall `anonymous namespace'::ScriptLoaderRunnable::*)(void),void,1>::Run() Line 383	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=false, bool * result=0x02b5f80c) Line 622	C++
 	xul.dll!NS_ProcessPendingEvents(nsIThread * thread=0x02763501, unsigned int timeout=0) Line 188	C++
 	xul.dll!MetroAppShell::DispatchAllGeckoEvents() Line 255	C++
 	xul.dll!MetroAppShell::ProcessOneNativeEventIfPresent() Line 296	C++
 	xul.dll!MetroAppShell::ProcessNextNativeEvent(bool mayWait=true) Line 320	C++
 	xul.dll!nsBaseAppShell::DoProcessNextNativeEvent(bool mayWait=true, unsigned int recursionDepth=0) Line 142	C++
 	xul.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00000014, bool mayWait=false, unsigned int recursionDepth=0) Line 295	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x02b5f8b3) Line 598	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x027635f8, bool mayWait=true) Line 238	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x0278ca30) Line 124	C++
 	xul.dll!MessageLoop::RunHandler() Line 214	C++
 	xul.dll!MessageLoop::Run() Line 188	C++
 	xul.dll!nsBaseAppShell::Run() Line 163	C++
 	xul.dll!MetroAppShell::Run() Line 195	C++
 	xul.dll!nsAppStartup::Run() Line 269	C++
 	xul.dll!XREMain::XRE_mainRun() Line 3886	C++
 	xul.dll!nsLocalFile::Release() Line 987	C++
 	xul.dll!nsQueryInterface::operator()(const nsID & aIID={...}, void * * answer=0x00000000) Line 19	C++
 	xul.dll!nsCOMPtr_base::assign_from_qi(const nsQueryInterface qi={...}, const nsID & iid={...}) Line 56	C++

Comment 21

[Jim Mathies [:jimm]]

Hmm, I can't reproduce now.

Comment 22

[Jim Mathies [:jimm]]

Attachment: fix

I don't remember why I added this condition. Hopefully it will come to me, in the mean time I'm going to run with this in my queue for a bit. I suspect I put this in because originally I forced a crash when re-entering ProcessOneNativeEventIfPresent, which was and still is unexpected (bug 914331). That got changed due to random test failures, and I think this code was designed to avoid it as well.

Comment 23

[Jim Mathies [:jimm]]

Attachment: patch

Comment 24

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 8337750 [details] [diff] [review]
patch

Review of attachment 8337750 [details] [diff] [review]:
-----------------------------------------------------------------

I'm still not able to reproduce this issue using the STR in comment 8 or the astronomy demo in the bug URL, so I can't tell if this patch has made things any better. Also, this bug has been around a lot longer than the code being removed in this patch, so it's surprising that removing that code fixes the issue.

That being said, I don't see anything wrong with this patch, and I'm always happy to see code get removed, so r+!

Comment 25

[Jim Mathies [:jimm]]

Attachment: export patch

Thanks!

Comment 26

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/fx-team/rev/50a0bc645e1b

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/50a0bc645e1b

Comment 28

[Rodrigo Silveira [:rsilveira] [:rodms]]

This bug was regressed by bug 944215. As per mbrubeck's suggestion, needinfoing bbondy. Does something stands out to you?

Comment 29

[Rodrigo Silveira [:rsilveira] [:rodms]]

I'll open a new bug for the regression.

Comment 30

[Marco Mucci [:MarcoM]]

Thanks Rodrigo.

(In reply to Rodrigo Silveira [:rsilveira] from comment #29)
> I'll open a new bug for the regression.