1164473 - Scroll via touch stop working after pen is activated

Status: RESOLVED FIXED

(Firefox :: Untriaged, defect)

Description

[Dmitriy Barkalov]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150415140819

Steps to reproduce:

1. Remove FF profile in user directory
2. Open nightly build https://treeherder.mozilla.org/#/jobs?repo=try&revision=891a9c2c9ab6 (win 8 opt)
3. Enable pointer events (set dom.w3c_pointer_events.enabled=true; dom.w3c_touch_events.enabled=1; layout.css.touch_action.enabled=true; layers.async-pan-zoom.enabled=true)
4. Open page with scroll (e.g. https://www.google.ru/?gws_rd=ssl#newwindow=1&q=nightly&spell=1)
5. Check scroll is working (can scroll page down via touch)
6. Click via pen inside Nightly window


Actual results:

Can't scroll page via touch anymore. Swipe up and down on the page doesn't scroll it.


Expected results:

Scroll should work as in step 5

Comment 1

[Maksim Lebedev]

Some investigation:
1. Normal working process with touches:
   [TOUCH]: nsWindow::OnTouch() -> nsWindow::DispatchMouseEvent() (compatible mouse event)
2. Some actions with PEN device:
   [PEN]: InkCollector::Initialize()
3. Abnormal working process with touches after InkCollector initialization:
   [TOUCH]: nsWindow::DispatchMouseEvent() (compatible mouse event)
Look's like, actions with PEN suppress normal touch events detection.

Comment 2

[Maksim Lebedev]

Some investigation:
After InkCollector activation looks like all touches are detected like pen actions. (But compatible mouse event have different flags for pen and real touch actions in that case). Maybe there is some HANDLE which was opened at the start of pen actions and we should close it at the end of pen actions. But what is that HANDLE and where is it?

Comment 3

[Maksim Lebedev]

> IInkRecognizers=CoCreateInstance(CLSID_InkRecognizers);
> IInkRecognizer=IInkRecognizers->GetDefaultRecognizer(0);
> IInkRecognizerContext=IInkRecognizer->CreateRecognizerContext();
> IInkRecognizerContext->Recognize() / EndInkInput();
Unfortunately, it does not help.

Comment 4

[Maksim Lebedev]

Attachment: pen_touch_issue.diff

+ Added InkCollector::ClearTarget()
- Removed MOZ_WM_PEN_LEAVES_HOVER_OF_DIGITIZER message
+ Changed  HWND mTargetWindow  ->  nsRefPtr<nsWindow> mTargetWindow

Suggestions and comments and objections are very welcome.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8608702 [details] [diff] [review]
pen_touch_issue.diff

This is so Windows specific that I don't have any useful feedback to this, except that why there are couple of lines commented out code
+    //LPARAM pos = lParamToClient(::GetMessagePos()); and
+    //nsIDOMMouseEvent::MOZ_SOURCE_PEN

Comment 6

[Maksim Lebedev]

Attachment: pen_touch_issue_ver2.diff

+ Returned using MOZ_WM_PEN_LEAVES_HOVER_OF_DIGITIZER message
+ Changed using PostMessage  ->  SendMessage.
- Returned nsRefPtr<nsWindow> mTargetWindow  ->  HWND mTargetWindow

Suggestions and comments and objections are very welcome.

Comment 7

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=19c7fe4d7308

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 8610180 [details] [diff] [review]
pen_touch_issue_ver2.diff

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

::: widget/windows/InkCollector.cpp
@@ +149,5 @@
> +void InkCollector::ClearTarget()
> +{
> +  if (mTargetWindow && mInkCollector) {
> +    Enable(false);
> +    if (S_OK == mInkCollector->put_hWnd(0)) {

nit - lets use SUCCEEDED() instead of a direct compare to S_OK.

Comment 9

[Maksim Lebedev]

(In reply to Jim Mathies [:jimm] from comment #8)
> nit - lets use SUCCEEDED() instead of a direct compare to S_OK.
According to specification of InkCollector and definition of SUCCEEDED macro
it looks like it is not good, because all errors will be interpret as true values.
So I would prefered have explicit comparison with S_OK at that place.

Comment 10

[Maksim Lebedev]

If there are no objections, I put checkin-needed flag.

Comment 11

[Pulsebot]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cc85865e7478

Comment 13

[Jim Mathies [:jimm]]

(In reply to Maksim Lebedev from comment #9)
> (In reply to Jim Mathies [:jimm] from comment #8)
> > nit - lets use SUCCEEDED() instead of a direct compare to S_OK.
> According to specification of InkCollector and definition of SUCCEEDED macro
> it looks like it is not good, because all errors will be interpret as true
> values.
> So I would prefered have explicit comparison with S_OK at that place.

Huh? How can SUCCEEDED interpret a com failure code as success?

Comment 14

[Dmitriy Barkalov]

(In reply to Maksim Lebedev from comment #7)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=19c7fe4d7308

Issue fixed on this build

Comment 15

[Maksim Lebedev]

(In reply to Jim Mathies [:jimm] from comment #13)
> Huh? How can SUCCEEDED interpret a com failure code as success?
I saw 2150105093(0x80280005) and 2147942487(0x80070057) as results of errors.
Looks like we can make such changes. If no objections, like minor changes in any future bug.

Comment 16

[Maksim Lebedev]

Some additional comments:
Remember that some stylus drivers block touch input while in use; this helps in cases where users rest their hands on the screen while using the stylus. In general, assume that stylus input takes priority.
https://software.intel.com/en-us/articles/mixing-stylus-and-touch-input-on-windows-8