959792 - On the nav bar edit, can't drag selection monocles for bad domains

Status: VERIFIED FIXED

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

Description

[Jim Mathies [:jimm]]

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

Comment 1

[Jim Mathies [:jimm]]

Attachment: fix v.1

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.

Comment 2

[Jim Mathies [:jimm]]

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.

Comment 3

[Matt Brubeck (:mbrubeck)]

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.

Comment 4

[Jim Mathies [:jimm]]

(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.

Comment 5

[Jim Mathies [:jimm]]

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.

Comment 6

[Jim Mathies [:jimm]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Matt Brubeck (:mbrubeck)]

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

Comment 9

[Manuela Muntean [Away]]

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