Jim Mathies [:jimm] Assignee
	Description • 11 years ago

Strange bug, str:

1) navigate to www.google.com
2) tab the urlbar edit
3) drag the selection monocles

result: works

4) navigate to www.gone.bz (bad domain)
5) tap the edit
6) drag the selection monocles

result: monocles can be dragged, disapear sometimes

	Jim Mathies [:jimm] Assignee
	Comment 1 • 11 years ago

Interesting bug to track down. Started up in the selection code but ultimately made my way down here after noticing the MozMouseHittest message we get in input.js wasn't getting sent for about pages.

mChromeHitTestCacheForTouch = (apzIntersect && HitTestChrome(pt));

If apzIntersect is false, mChromeHitTestCacheForTouch is false, which isn't right.

	Jim Mathies [:jimm] Assignee
	Comment 2 • 11 years ago

I wonder if we should rename mChromeHitTestCacheForTouch to something along the line of "mNonApzTargetForTouch". Since ApzHitTest may return false for content that doesn't support scrolling or zooming.

	Matt Brubeck (:mbrubeck)
	Comment 3 • 11 years ago

Comment on attachment 8360491 [details] [diff] [review]
fix v.1

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

Argh, I thought I fixed this at one point but it must have been in a patch that I discarded or backed out.  :(

::: widget/windows/winrt/MetroInput.cpp
@@ -1192,5 @@
>    if (mCancelable && event->message == NS_TOUCH_START) {
>      nsRefPtr<Touch> touch = event->touches[0];
>      LayoutDeviceIntPoint pt = LayoutDeviceIntPoint::FromUntyped(touch->mRefPoint);
> -    bool apzIntersect = mWidget->ApzHitTest(mozilla::ScreenIntPoint(pt.x, pt.y));
> -    mChromeHitTestCacheForTouch = (apzIntersect && HitTestChrome(pt));

If you want to preserve the original structure, you could just correct this to:

  mChromeHitTestCacheForTouch = (!apzIntersect || HitTestChrome(pt));

Whichever version you find clearer is fine.

	Jim Mathies [:jimm] Assignee
	Comment 4 • 11 years ago

(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> Comment on attachment 8360491 [details] [diff] [review]
> fix v.1
> 
> Review of attachment 8360491 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Argh, I thought I fixed this at one point but it must have been in a patch
> that I discarded or backed out.  :(
> 
> ::: widget/windows/winrt/MetroInput.cpp
> @@ -1192,5 @@
> >    if (mCancelable && event->message == NS_TOUCH_START) {
> >      nsRefPtr<Touch> touch = event->touches[0];
> >      LayoutDeviceIntPoint pt = LayoutDeviceIntPoint::FromUntyped(touch->mRefPoint);
> > -    bool apzIntersect = mWidget->ApzHitTest(mozilla::ScreenIntPoint(pt.x, pt.y));
> > -    mChromeHitTestCacheForTouch = (apzIntersect && HitTestChrome(pt));
> 
> If you want to preserve the original structure, you could just correct this
> to:
> 
>   mChromeHitTestCacheForTouch = (!apzIntersect || HitTestChrome(pt));
> 
> Whichever version you find clearer is fine.

wfm! I'm going to update the variable name as well.

	Jim Mathies [:jimm] Assignee
	Comment 5 • 11 years ago

This is also going to need merging with nick's patches on inbound. I'm going to let those merge to mc and land this tomorrow.

	Jim Mathies [:jimm] Assignee
	Comment 6 • 11 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/077f80707004

	Matt Brubeck (:mbrubeck)
	Comment 8 • 11 years ago

Landed the original attachment on Aurora, since the patch that landed on m-c depends on m-c-only changes:
https://hg.mozilla.org/releases/mozilla-aurora/rev/4308bd949a96

	Manuela Muntean [Away]
	Comment 9 • 11 years ago

Verified as fixed for iteration #22, with both latest Nightly and Aurora on Surface pro 2 with Windows 8.1 64-bit.