Closed
Bug 914331
Opened 11 years ago
Closed 11 years ago
[MP] Defect - Soft keyboard requires two taps on a form inputs to display
Categories
(Core Graveyard :: Widget: WinRT, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla26
People
(Reporter: jimm, Assigned: jimm)
References
Details
(Whiteboard: [preview] feature=defect c=content_features u=metro_firefox_user p=2)
Attachments
(2 files, 1 obsolete file)
7.83 KB,
patch
|
TimAbraldes
:
review+
|
Details | Diff | Splinter Review |
990 bytes,
patch
|
bbondy
:
review+
bbondy
:
feedback+
|
Details | Diff | Splinter Review |
Win 8.1 specific. In tapping form inputs I'm not getting the soft keyboard on the first tab, only the second. Also, tapping outside an input requires two taps to hide the keyboard.
Updated•11 years ago
|
Blocks: metrov1backlog
Summary: Soft keyboard requires two taps on a form inputs to display → Defect - Soft keyboard requires two taps on a form inputs to display
Whiteboard: [preview-triage] → [preview-triage] feature=defect c=tbd u=tbd p=0
![]() |
Assignee | |
Updated•11 years ago
|
No longer blocks: metrov1backlog
Summary: Defect - Soft keyboard requires two taps on a form inputs to display → Soft keyboard requires two taps on a form inputs to display
Whiteboard: [preview-triage] feature=defect c=tbd u=tbd p=0 → [preview-triage][8.1]
![]() |
Assignee | |
Updated•11 years ago
|
Summary: Soft keyboard requires two taps on a form inputs to display → Defect - Soft keyboard requires two taps on a form inputs to display
Whiteboard: [preview-triage][8.1] → [preview-triage][8.1]feature=defect c=tbd u=tbd p=0
![]() |
Assignee | |
Updated•11 years ago
|
Blocks: metrov1backlog
![]() |
Assignee | |
Comment 1•11 years ago
|
||
Appears to be some sort of a timing issue with accessibility.
![]() |
Assignee | |
Updated•11 years ago
|
Whiteboard: [preview-triage][8.1]feature=defect c=tbd u=tbd p=0 → [preview][8.1]feature=defect c=tbd u=tbd p=0
Updated•11 years ago
|
Blocks: MetroPreviewRelease
Summary: Defect - Soft keyboard requires two taps on a form inputs to display → [MP] Defect - Soft keyboard requires two taps on a form inputs to display
![]() |
Assignee | |
Comment 2•11 years ago
|
||
I was concerned that the async events work might have broken this, but it doesn't appear to. We query after all events are processed, for some reason accessibility returns the document instead of the input element -
mozilla::widget::winrt::MetroInput::OnPointerEntered
mozilla::widget::winrt::MetroInput::OnPointerPressed
mozilla::widget::winrt::MetroInput::OnPointerReleased
mozilla::widget::winrt::MetroInput::OnTapped
mozilla::widget::winrt::MetroInput::HandleSingleTap
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::UIABridge::get_ProviderOptions
UIABridge::GetPropertyValue: idProp=UIA_NativeWindowHandlePropertyId
mozilla::widget::winrt::UIABridge::Navigate
UIABridge::Navigate NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::get_ProviderOptions
mozilla::widget::winrt::UIABridge::get_ProviderOptions
mozilla::widget::winrt::UIABridge::Navigate
UIABridge::Navigate NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::GetFocus
name: http://mathies.com/mozilla/forms.html
description:
Focus element flags: editable:0 focusable:1 readonly:1
![]() |
Assignee | |
Comment 3•11 years ago
|
||
Actually after testing a bit I think this was caused by bug 907410, and I can reproduce it on 8.0 as well. Accessibility updates after the gesture recognizer generates an ontapped event which we fire from HandleSingleTap. Looks like these events don't get delivered by the time Windows comes in looking for the focused element.
Blocks: 907410
Whiteboard: [preview][8.1]feature=defect c=tbd u=tbd p=0 → [preview] feature=defect c=tbd u=tbd p=0
![]() |
Assignee | |
Comment 4•11 years ago
|
||
More detailed log output -
mozilla::widget::winrt::MetroInput::OnPointerEntered
mozilla::widget::winrt::MetroInput::OnPointerPressed
dispatching 5200
mozilla::widget::winrt::MetroInput::OnPointerReleased
mozilla::widget::winrt::MetroInput::OnTapped
mozilla::widget::winrt::MetroInput::HandleSingleTap
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
mozilla::widget::winrt::MetroInput::DispatchAsyncEventIgnoreStatus
dispatching 5201
WM_GETOBJECT
mozilla::widget::winrt::UIABridge::get_ProviderOptions
UIABridge::GetPropertyValue: idProp=UIA_NativeWindowHandlePropertyId
mozilla::widget::winrt::UIABridge::Navigate
UIABridge::Navigate NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::get_ProviderOptions
mozilla::widget::winrt::UIABridge::get_ProviderOptions
mozilla::widget::winrt::UIABridge::Navigate
UIABridge::Navigate NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::GetFocus
name: http://mathies.com/mozilla/forms.html
description:
Focus element flags: editable:0 focusable:1 readonly:1
mozilla::widget::winrt::UIATextElement::ClearFocus
UIABridge::GetPropertyValue: idProp=UIA_IsControlElementPropertyId
UIABridge::GetPropertyValue: idProp=UIA_NativeWindowHandlePropertyId
mozilla::widget::winrt::UIABridge::get_BoundingRectangle
UIABridge::GetPropertyValue: idProp=UIA_ControlTypePropertyId
UIABridge::GetPropertyValue: idProp=49209
UIABridge: Unhandled property
UIABridge::GetPropertyValue: idProp=49213
UIABridge: Unhandled property
mozilla::widget::winrt::UIABridge::GetPatternProvider
UIABridge::GetPatternProvider=10014
mozilla::widget::winrt::UIABridge::GetPatternProvider
UIABridge::GetPatternProvider=10029
UIABridge::GetPropertyValue: idProp=UIA_IsPasswordPropertyId
dispatching 5202
mozilla::widget::winrt::MetroInput::OnPointerExited
dispatching 300
dispatching 302
dispatching 301
dispatching 300
mozilla::widget::winrt::UIABridge::FocusChangeEvent
![]() |
Assignee | |
Comment 5•11 years ago
|
||
Assignee: nobody → jmathies
Comment 6•11 years ago
|
||
Hey Jim, what point value would you like to assign to this defect? I'll add it to Iteration #15.
Flags: needinfo?(jmathies)
![]() |
Assignee | |
Updated•11 years ago
|
Flags: needinfo?(jmathies)
Whiteboard: [preview] feature=defect c=tbd u=tbd p=0 → [preview] feature=defect c=tbd u=tbd p=2
Updated•11 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
![]() |
Assignee | |
Updated•11 years ago
|
Component: Input → Widget: WinRT
Product: Firefox for Metro → Core
![]() |
Assignee | |
Comment 7•11 years ago
|
||
Unfortunate that we have to do this since it'll cause problems if and when we try to completely split the threads. But we have to have an up to date dom by the time uia comes in looking for focus information.
Attachment #802354 -
Attachment is obsolete: true
Attachment #802548 -
Flags: review?(tabraldes)
Comment 8•11 years ago
|
||
Comment on attachment 802548 [details] [diff] [review]
dispatch pending events v.1
Review of attachment 802548 [details] [diff] [review]:
-----------------------------------------------------------------
It took me a little while to figure out how this is working. Hopefully we'll have some time after the v1 launch to reorganize our event processing
Attachment #802548 -
Flags: review?(tabraldes) → review+
![]() |
Assignee | |
Comment 9•11 years ago
|
||
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #8)
> Comment on attachment 802548 [details] [diff] [review]
> dispatch pending events v.1
>
> Review of attachment 802548 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> It took me a little while to figure out how this is working. Hopefully we'll
> have some time after the v1 launch to reorganize our event processing
Yeah. Once we are rolling out we just make the time.
This bug brought to light a big issue in the thread splitting work for me - accessibility. That module will be accessed via the winrt main thread. It is also heavily embedded in the dom and I doubt it is multi-thread safe. This may prevent us from splitting gecko / winrt input threads.
Comment 10•11 years ago
|
||
> This bug brought to light a big issue in the thread splitting work for me -
> accessibility. That module will be accessed via the winrt main thread. It is
> also heavily embedded in the dom and I doubt it is multi-thread safe. This
> may prevent us from splitting gecko / winrt input threads.
yeah, you can only use a11y on the same thread as the DOM / layout, and I don't see how we could reasonable change that requirement without making the dom / layout thread safe. We could of course proxy things between threads if necessary and it isn't too slow for your application.
![]() |
Assignee | |
Comment 11•11 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #10)
> > This bug brought to light a big issue in the thread splitting work for me -
> > accessibility. That module will be accessed via the winrt main thread. It is
> > also heavily embedded in the dom and I doubt it is multi-thread safe. This
> > may prevent us from splitting gecko / winrt input threads.
>
> yeah, you can only use a11y on the same thread as the DOM / layout, and I
> don't see how we could reasonable change that requirement without making the
> dom / layout thread safe. We could of course proxy things between threads
> if necessary and it isn't too slow for your application.
Was wondering about that. I think it would be (more) messy proxying IAccessible, IAccessibleEx + the parts of UIA metro uses. Might be simpler to go for a full UIA implementation and just proxy that.
Comment 12•11 years ago
|
||
I was thinking about IAccessibleEx as temporary solution, as a bridge between a11y core and UIA. Later I wanted to get a mapping from a11y core to full UIA.
Comment 13•11 years ago
|
||
Backed out along with bug 914829 for intermittent mochitest-mc crashes.
https://hg.mozilla.org/integration/fx-team/rev/4be36d56d323
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734694&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734168&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734386&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734018&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734103&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734434&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734211&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=27728411&tree=Fx-Team
Comment 14•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #11)
> (In reply to Trevor Saunders (:tbsaunde) from comment #10)
> > > This bug brought to light a big issue in the thread splitting work for me -
> > > accessibility. That module will be accessed via the winrt main thread. It is
> > > also heavily embedded in the dom and I doubt it is multi-thread safe. This
> > > may prevent us from splitting gecko / winrt input threads.
> >
> > yeah, you can only use a11y on the same thread as the DOM / layout, and I
> > don't see how we could reasonable change that requirement without making the
> > dom / layout thread safe. We could of course proxy things between threads
> > if necessary and it isn't too slow for your application.
>
> Was wondering about that. I think it would be (more) messy proxying
> IAccessible, IAccessibleEx + the parts of UIA metro uses. Might be simpler
> to go for a full UIA implementation and just proxy that.
Well, the other thing we could do is proxy our internal api, which may be useful for e10s, but I really haven't thought about it much.
![]() |
Assignee | |
Comment 15•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #13)
> Backed out along with bug 914829 for intermittent mochitest-mc crashes.
> https://hg.mozilla.org/integration/fx-team/rev/4be36d56d323
>
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734386&full=1&branch=fx-team#error0
This work enabled the "crash on purpose" code for re-entrant calls to ProcessOneNativeEventIfPresent, which is what most of these are.
Also seems our crash stack walker can't go deep enough to see what's triggering this. :/
![]() |
Assignee | |
Comment 16•11 years ago
|
||
interesting -
17:52:45 INFO - 8 xul.dll!XPCWrappedNative::CallMethod(XPCCallContext &,XPCWrappedNative::CallMode) [XPCWrappedNative.cpp:35a772328e24 : 2110 + 0x75d]
17:52:45 INFO - eip = 0x73d1bfa1 esp = 0x0305c8a0 ebp = 0x0305c9e8
17:52:45 INFO - Found by: call frame info
17:52:45 INFO - 9 xul.dll!xpc::JSMemoryMultiReporter::CollectReports(nsDataHashtable<nsUint64HashKey,nsCString> *,nsDataHashtable<nsUint64HashKey,nsCString> *,nsIMemoryMultiReporterCallback *,nsISupports *) [XPCJSRuntime.cpp:35a772328e24 : 2692 + 0x3]
17:52:45 INFO - eip = 0x73d1010a esp = 0x0305c900 ebp = 0x0305c9e8
17:52:45 INFO - Found by: stack scanning
17:52:45 INFO - 10 xul.dll!XPC_WN_CallMethod(JSContext *,unsigned int,JS::Value *) [XPCWrappedNativeJSOps.cpp:35a772328e24 : 1307 + 0xc]
So my guess is we are processing a NativeCallback and drop into a js event dispatch loop.
![]() |
Assignee | |
Comment 17•11 years ago
|
||
Ted, curious if you can help me here, I'm having a hard time making sense of this stack -
https://tbpl.mozilla.org/php/getParsedLog.php?id=27734168&full=1&branch=fx-team#error0
Specifically this frame -
xul.dll!xpc::JSMemoryMultiReporter::CollectReports(nsDataHashtable<nsUint64HashKey,nsCString> *,nsDataHashtable<nsUint64HashKey,nsCString> *,nsIMemoryMultiReporterCallback *,nsISupports *) [XPCJSRuntime.cpp:a1a846de1a8a : 2692 + 0xc]
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSRuntime.cpp#2692
which points to no mans land. I also don't see where CollectReports might invoke nsThread::ProcessNextEvent?
Flags: needinfo?(ted)
Comment 18•11 years ago
|
||
Note the "Found by: stack scanning", which means that the stack walker is kind of guessing here. The rest of that stack looks pretty sane though, so I'd just ignore that one frame. I think you're just seeing JS spin the event loop.
Flags: needinfo?(ted)
Comment 19•11 years ago
|
||
If you can reproduce this on try, you can make your build upload PDB files:
https://wiki.mozilla.org/ReleaseEngineering/TryServer#Getting_debug_symbols
and then ask someone in releng to grab the minidump from the slave for you (in the log you'll see "Saved dump as...")
![]() |
Assignee | |
Comment 20•11 years ago
|
||
Attachment #803659 -
Flags: feedback?(netzen)
Updated•11 years ago
|
Attachment #803659 -
Flags: feedback?(netzen) → feedback+
![]() |
Assignee | |
Comment 21•11 years ago
|
||
Comment on attachment 803659 [details] [diff] [review]
don't crash, log instead
Longer term I think we need override more of nsBaseAppShell, but I'm not interested in doing that three days before aurora uplift. So lets land this work around which is what we've been running with for the last few weeks (prior to the landing here) and I'll file a follow up on more app shell event work.
Attachment #803659 -
Flags: review?(netzen)
Updated•11 years ago
|
Attachment #803659 -
Flags: review?(netzen) → review+
![]() |
Assignee | |
Comment 22•11 years ago
|
||
Comment 23•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Updated•11 years ago
|
Whiteboard: [preview] feature=defect c=tbd u=tbd p=2 → [preview] feature=defect c=content_features u=metro_firefox_user p=2
Updated•11 years ago
|
OS: Windows 8 Metro → Windows 8.1
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•