Closed Bug 863676 Opened 11 years ago Closed 11 years ago

Sporadic display issue with soft keyboard

Categories

(Core Graveyard :: Widget: WinRT, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: jimm, Assigned: jimm)

References

Details

(Keywords: qawanted)

Attachments

(5 files, 3 obsolete files)

There is a random display issue with the skb, which appears to have something to do with UIA or widget or maybe it's just some weird timing issue. Here's a UIA trace after a tap on an in-content text input:

mozilla::widget::winrt::UIABridge::GetPropertyValue
mozilla::widget::winrt::UIABridge::Navigate
NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::Navigate
NavigateDirection_Parent
mozilla::widget::winrt::UIABridge::GetFocus
Focus element flags: 1 1 0
mozilla::widget::winrt::UIATextElement::SetFocusInternal
mozilla::widget::winrt::UIATextElement::GetPropertyValue
mozilla::widget::winrt::UIATextElement::GetPropertyValue
mozilla::widget::winrt::UIATextElement::Navigate
mozilla::widget::winrt::UIATextElement::GetPropertyValue
UIATextElement::GetPropertyValue: idProp=30016
mozilla::widget::winrt::UIATextElement::GetPropertyValue
mozilla::widget::winrt::UIATextElement::Navigate
mozilla::widget::winrt::UIABridge::GetPropertyValue
mozilla::widget::winrt::UIABridge::GetPropertyValue
mozilla::widget::winrt::UIABridge::Navigate
mozilla::widget::winrt::UIABridge::Navigate
mozilla::widget::winrt::UIABridge::GetPropertyValue
mozilla::widget::winrt::UIATextElement::get_BoundingRectangle
mozilla::widget::winrt::UIATextElement::GetPropertyValue
UIATextElement::GetPropertyValue: idProp=30003
mozilla::widget::winrt::UIATextElement::GetPropertyValue
UIATextElement::GetPropertyValue: idProp=49228
UIATextElement: Unhandled property
mozilla::widget::winrt::UIATextElement::GetPatternProvider
UIATextElement::GetPatternProvider=10014
** TextPattern requested from element.
mozilla::widget::winrt::UIATextElement::GetPatternProvider
UIATextElement::GetPatternProvider=10029
mozilla::widget::winrt::UIATextElement::GetPropertyValue
UIATextElement::GetPropertyValue: idProp=30019
mozilla::widget::winrt::UIATextElement::GetPatternProvider
UIATextElement::GetPatternProvider=10002
** ValuePattern requested from element.
mozilla::widget::winrt::UIATextElement::GetPatternProvider
UIATextElement::GetPatternProvider=10002
** ValuePattern requested from element.
mozilla::widget::winrt::UIATextElement::get_IsReadOnly
mozilla::widget::winrt::UIABridge::FocusChangeEvent
Focus element flags: 1 1 0
mozilla::widget::winrt::UIATextElement::AnnounceFocus
mozilla::widget::winrt::UIATextElement::GetPropertyValue
mozilla::widget::winrt::UIATextElement::GetPropertyValue
mozilla::widget::winrt::UIATextElement::Navigate
mozilla::widget::winrt::FrameworkView::OnSoftkeyboardShown
************* metro_softkeyboard_shown 
mozilla::widget::winrt::FrameworkView::OnSoftkeyboardHidden
************* metro_softkeyboard_hidden 


I have not been able to track down the cause of this. The keyboard hidden event fires for a reason that isn't apparent in widget logging. I've managed to trap the callback event in the debugger but unfortunately the stack isn't of much use since it's sent to metrofx async over com, so whatever triggers it is off the stack.

I'm at a loss as to the cause.

I've also tested with TSF disabled and can still reproduce, so I'm pretty sure that module isn't involved.

This may be due to the nature of UIABridge which isn't a real UIA implementation. It's just a light weight UIA wrapper that holds a single IRawElementProviderSimple who's bounds change when the user moves around the document.
What we might consider is to go ahead and implement bug 762817. That would take some work, but it would allow us to latch into our existing accessibility layer fully and remove the temporary bridge code.
Attached file text input test case (obsolete) —
Blocks: 889997
No longer blocks: 889997
Blocks: 833305
Blocks: 855297
No longer blocks: 833305
Assignee: nobody → jmathies
Blocks: metrov1it12
Whiteboard: p=8
Hey Jim, no points required.  This is a dependent work item part of Story Bug 855297.
No longer blocks: 855297, metrov1it12
Whiteboard: p=8
No longer blocks: 861945
Blocks: 855297
Not sure why this blocked bug 861945, they two are unrelated.
Should we put time into trying to nail down our remaining keyboard display problem for 26?
Flags: needinfo?(asa)
(In reply to Jim Mathies [:jimm] from comment #10)
> Should we put time into trying to nail down our remaining keyboard display
> problem for 26?

Yes, please. Let's add this to the Preview list. Thanks, Jim.
Flags: needinfo?(asa)
Hey Jim, are you taking the full story (Bug 855297) or just this work item for Iteration #12?  If it's the whole story, I'll add it to the iteration and the MP list.  If it's just the work item, no need to do anything else.  Thanks.
Flags: needinfo?(jmathies)
(In reply to Marco Mucci [:MarcoM] from comment #12)
> Hey Jim, are you taking the full story (Bug 855297) or just this work item
> for Iteration #12?  If it's the whole story, I'll add it to the iteration
> and the MP list.  If it's just the work item, no need to do anything else. 
> Thanks.

Those others should probably be triaged independently. I'm only planning on working on this bug.
Flags: needinfo?(jmathies)
Attached patch debug patchSplinter Review
Attached patch wip (obsolete) — Splinter Review
Attached patch wip (obsolete) — Splinter Review
Attachment #784985 - Attachment is obsolete: true
Attached file textboxes.html
Attachment #739534 - Attachment is obsolete: true
Attached file twotextareas.html
Attached file forms.html
Hey Juan, I think I may have a good fix for this. I've been testing on a surface pro with this try build - 

https://tbpl.mozilla.org/?tree=Try&showall=0&rev=12e7404f9314

without issue. Curious if QA might have the time to take this build for a spin on some other hardware to see if the problem persists?
Flags: needinfo?(jbecerra)
Keywords: qawanted
I'll take a look now with the three devices I have (Surface Pro, Dell XPS, Iconia W3).
Flags: needinfo?(jbecerra)
(In reply to juan becerra [:juanb] from comment #21)
> I'll take a look now with the three devices I have (Surface Pro, Dell XPS,
> Iconia W3).

Thanks!
I have been testing this on the three machines listed in comment #21 with some of the examples here, as well just playing around with only the skb, and I haven't seen any issues with the software keyboard itself.
Attached patch patch v.1Splinter Review
A lot of this is logging and some refining of the UIABridge code. The key change here is the added WM_GETOBJECT handler in MetroWidget.
Attachment #784990 - Attachment is obsolete: true
Attachment #785138 - Flags: review?(netzen)
Comment on attachment 785138 [details] [diff] [review]
patch v.1

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

::: widget/windows/winrt/FrameworkView.cpp
@@ +367,5 @@
>      return S_OK;
>    sKeyboardIsVisible = false;
>    memset(&sKeyboardRect, 0, sizeof(Rect));
>    MetroUtils::FireObserver("metro_softkeyboard_hidden");
> +  aArgs->put_EnsuredFocusedElementInView(true);

Can aArg ever be passed in a nullptr value?
Attachment #785138 - Flags: review?(netzen) → review+
(In reply to Brian R. Bondy [:bbondy] from comment #25)
> Comment on attachment 785138 [details] [diff] [review]
> patch v.1
> 
> Review of attachment 785138 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/winrt/FrameworkView.cpp
> @@ +367,5 @@
> >      return S_OK;
> >    sKeyboardIsVisible = false;
> >    memset(&sKeyboardRect, 0, sizeof(Rect));
> >    MetroUtils::FireObserver("metro_softkeyboard_hidden");
> > +  aArgs->put_EnsuredFocusedElementInView(true);
> 
> Can aArg ever be passed in a nullptr value?

Don't think winrt would send us a null args param.
https://hg.mozilla.org/mozilla-central/rev/859a06fa227c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
OS: Windows 8 Metro → Windows 8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: