819223 - Work - Touch Screen, immediately crashes

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Input, defect)

Description

[matthewdark]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:20.0) Gecko/20121206 Firefox/20.0
Build ID: 20121206040206

Steps to reproduce:

Touched the screen with my finger, doesn't even need to be an element of the browser or the web page.  Scrolling would do it, and I can't sweep in the top or bottom.  Sweeping in the sides works (Windows owns that part of the UI I guess?) Really any touch, just a tap, not even a scroll

Whenever I use the touch screen, specifically finger touch, the laptop differentiates between finger and pen input, and pen is fine


Actual results:

Immediate crash.

Nightly immediately crashes.

Report ID
bp-3b3614cf-e0f1-4d77-abbf-0f0f8212120712/6/20128:37 PMbp-b2e86019-435c-4bcb-a588-45695212120712/6/20128:36 PMbp-5eb7bb60-6694-44ef-aed1-98a5e212120712/6/20128:35 PMbp-a00a87af-25d7-4553-a45d-e6007212120712/6/20128:34 PM


Expected results:

page should have scrolled, element selected, or top/bottom metro elements popped up.

Comment 1

[Scoobidiver (away)]

Here are linked crash IDs:
bp-3b3614cf-e0f1-4d77-abbf-0f0f82121207
bp-b2e86019-435c-4bcb-a588-456952121207
bp-5eb7bb60-6694-44ef-aed1-98a5e2121207
bp-a00a87af-25d7-4553-a45d-e60072121207

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Based on where it's crashing, it seems like Firefox is receiving a PointerMoved event without ever having received a PointerPressed event.  I believe that should be impossible, but maybe for certain hardware Windows doesn't send us PointerPressed events?

Matthew: What kind of hardware are you running on?  Also what version of Windows 8 are you running?  Does this issue reproduce consistently or can you sometimes touch the screen without crashing?

Comment 3

[Jim Mathies [:jimm]]

https://crash-stats.mozilla.com/report/list?product=MetroFirefox&query_search=signature&query_type=contains&reason_type=contains&date=12%2F07%2F2012%2022%3A07%3A47&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Awidget%3A%3Awinrt%3A%3AMetroInput%3A%3AOnPointerMoved%28ABI%3A%3AWindows%3A%3AUI%3A%3ACore%3A%3AICoreWindow*%2C%20ABI%3A%3AWindows%3A%3AUI%3A%3ACore%3A%3AIPointerEventArgs*%29

Yuan was getting this today a lot. #1 top crasher for metro. Can we add something to protect against this, maybe check that touchEntry pointer and bail if it's null?

Comment 4

[matthewdark]

Attachment: Lenovo X220T DxDiag

Just a dxdiag dump if anyone wants to see the hardware side.

Comment 5

[matthewdark]

Hardware is a Lenovo X220T.  I can't find a good name that sounds like the digitizer in my device manager except for some of the generic HID drivers.

I believe it's a Wacom digitizer?

It doesn't care if I scroll or touch/click in non-metro (desktop?) mode.

Comment 6

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Jim Mathies [:jimm] from comment #3)
> https://crash-stats.mozilla.com/report/
> list?product=MetroFirefox&query_search=signature&query_type=contains&reason_t
> ype=contains&date=12%2F07%2F2012%2022%3A07%3A47&range_value=1&range_unit=week
> s&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Awidget%3A%
> 3Awinrt%3A%3AMetroInput%3A%3AOnPointerMoved%28ABI%3A%3AWindows%3A%3AUI%3A%3AC
> ore%3A%3AICoreWindow*%2C%20ABI%3A%3AWindows%3A%3AUI%3A%3ACore%3A%3AIPointerEv
> entArgs*%29
> 
> Yuan was getting this today a lot. #1 top crasher for metro. Can we add
> something to protect against this, maybe check that touchEntry pointer and
> bail if it's null?

I'm hesitant to add a nullptr check without understanding why we're getting a PointerMoved event for a touchpoint that we don't know about.  Touch input is likely to break in unexpected ways if we continue finding ourselves in this situation but we just bail on attempting to process the PointerMoved.

If Windows is indeed sending us PointerMoved events for touches that we never received PointerPressed events for, then we'll have to treat the first PointerMoved as a PointerPressed.  On the other hand, if we're somehow losing track of the pointer between the PointerPressed and the PointerMoved, then we have to track down how that is happening.

I'm attempting to reproduce this locally and come up with a fix.

Comment 7

[matthewdark]

Just throwing this out there, I'm just a user, etc:

Windows looks like it handles my Pen and Touch as separate devices.  Pen input works fine, but an actual finger crashes it.  Maybe having so many pointing devices is throwing it off somehow

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v1 (WIP)

I'm not able to reproduce this bug, but if the problem is that Windows is sending us PointerMoved events without first sending us PointerPressed events, then something like this patch should fix the issue.

Comment 9

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

We were able to verify on Yuan's machine that updated drivers can solve this issue.  It seems that for certain hardware/drivers, Windows does not send us PointerPressed events; only PointerMoved and PointerReleased.

I've created a build that has the patch for this bug applied, and it's available at:
  http://people.mozilla.org/~tabraldes/FirefoxInstallerBug819223.exe

Matthew; are you able to install and test this build?

Comment 10

[matthewdark]

That resolved the specific issue I had.  There's still some unexpected behavior.

I can click on web page elements with a touch, and page back + page forward gestures work, as does sliding in from the top/bottom.

Page scrolling doesn't work, and pinching to zoom (Not even sure if that's a feature yet)

Thanks for the build, I'm not really equipped to for it.

Comment 11

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to matthewdark from comment #10)
> That resolved the specific issue I had.  There's still some unexpected
> behavior.
> 
> I can click on web page elements with a touch, and page back + page forward
> gestures work, as does sliding in from the top/bottom.

Great!
 
> Page scrolling doesn't work, and pinching to zoom (Not even sure if that's a
> feature yet)

Pinch to zoom isn't implemented yet.  For page scrolling; are you talking about just regular touch-the-page-and-move-your-finger-to-scroll-around or something else?

If the former, then I've got to work on this patch some more.  If the latter, that's because we don't have any scroll-a-whole-page functionality built in.

> Thanks for the build, I'm not really equipped to for it.

No problem! Glad it's an improvement over the crash experience

Comment 12

[matthewdark]

Regular page scrolling doesn't work then.  Just touching the body of the page wherever and moving my finger like I'd expect on a touch interface doesn't do anything.

While touching Nightly-metro I can see the trail following my finger that Windows 8 does in most desktop apps.... I just noticed, maybe it does that in non-metro apps?  I just verified it the trail still exists on the 'desktop' and whatever apps I'm running in there, but I don't have the same trail in IE-Metro or the start menu or MS-mahjong...  I tested several of the MS Metro apps and they all lack the finger trail.  I'd guess this indicates something about how Nightly is handling Metro input different from other metro apps?  (Maybe not, it could just be aesthetic, I can't test third party apps because my connection is badly limited tonight)

With my scroll wheel (actually a thinkpad eraser head) I can still move the page like I'd expect.

Pen input still works as expected.  It selects text and can right click.

Comment 13

[Jim Mathies [:jimm]]

Have you tied updating your drivers for Win8? I did a little searching around and found new ThinkPad X220 & X220i drivers for win8 here - 

http://support.lenovo.com/en_US/research/hints-or-tips/detail.page?&LegacyDocID=MIGR-77280

The Synaptic driver was released on Sept 21st - 

http://support.lenovo.com/en_US/downloads/detail.page?DocID=DS019164

Version 16.2.14.0 - [Important] Added support for Windows 8.

Comment 14

[Jim Mathies [:jimm]]

Also, lots of info here if you have haven't found this site yet - 

http://forums.lenovo.com/t5/Windows-8/bd-p/Windows_8

Comment 15

[Albert Armea]

I have the same issue reported by the original reporter - crashes on touch input, but not on mouse or pen input.

Comment 16

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Touch Test

I was planning to downgrade the firmware on my Samsung Series 7 slate to reproduce this issue locally but ended up making that machine unbootable.  I'll have to reinstall Windows today.

In the meantime, anyone who can reproduce this issue: Would you please install the build at [1], open the attached HTML file, drag your finger across the middle of the screen, then take a screenshot and attach it to this bug?

Also, how well does Metro IE respond to touch input on machines that reproduce this issue?  Are you able to touch-scroll?

[1] http://people.mozilla.org/~tabraldes/FirefoxInstallerBug819223.exe

Comment 17

[matthewdark]

Attachment: Touched center of screen dragged to top

As requested.  Specifically I touched from the center of the screen and dragged towards the top, as if I wanted to scroll down a page.  I couldn't detect any movement of the reference mark.

Comment 18

[matthewdark]

Metro-IE works fine on my X220T.

I do have the updated drivers, no other hardware problems. (Well my boot loader is all messed up but that's a thing I need to fix with Lenovo and MS :)

Comment 19

[matthewdark]

Attachment: Slide in works fine

Touch slide in from top or bottom of screen works as expected and brings out the app's menus.

Comment 20

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v2 (WIP)

Based on the behavior we've witnessed so far, I suspect that either the PointerPressed and PointerMoved events are swapped (we get one when we should be getting the other) or the pointerId values aren't consistent between events.  The patch I'm attaching fixes the issue if we are seeing swapped PointerPressed and PointerMoved events, but it also adds a bunch of logging that will tell me once and for all what the issue is.

I need someone who is experiencing this issue to do the following, then I'll be able to submit a patch that is better than guesswork :)

  0) Uninstall any previous versions of Firefox and set IE as your default browser (Control Panel -> "Set Your Default Programs" -> Internet Explorer -> "Set this program as default"
  1) Install the build at [1], run it and set Firefox as your default browser so that Firefox can run in Metro mode
  2) Create the NSPR_LOG_FILE environment variable, set its value to point to a text file on your desktop or somewhere convenient (Control Panel -> System -> "Advanced System Settings" -> "Environment Variables" -> "New" (in the "user variables" section)
  3) Open Metro Firefox
  4) Touch the center of the screen and swipe slowly to one direction
  5) Close Metro Firefox using the Win8 "close" gesture (drag from top of screen to bottom)
  7) Check the file that NSPR_LOG_FILE points to. It should contain entries like "touchstart, PointerId=xx" and "touchmove, PointerId=xx"
  8) Upload/attach that file to this bug so I can access it

[1] http://people.mozilla.org/~tabraldes/FirefoxInstallerBug819223.exe - this is an updated version of the previous build. It contains the patch that I currently am attaching to this bug.

Comment 21

[matthewdark]

Weird.  I can't get it to write anything to the log file.  I tried in %userprofile%\My Documents\note.txt and c:\note.txt and c:\

CMD reports:
NSPR_LOG_FILE=c:\note.txt
and I can edit it from notepad %NSPR_LOG_FILE%, so I'm pretty sure I have it set right.  So unless nightly isn't updating its env variables when I launch it from metro, I dunno what's up.

I've tried a couple locations and finally gave it c:\note.txt to make it as easy to parse as I could, and given Everyone full control of the file, so it should be able to access it.

It says the build date was 12-17 so it must be your latest version, maybe the patch didn't get patched? :)

Comment 22

[Jim Mathies [:jimm]]

> [1] http://people.mozilla.org/~tabraldes/FirefoxInstallerBug819223.exe -
> this is an updated version of the previous build. It contains the patch that
> I currently am attaching to this bug.

If you want you can avoid doing these yourself. Create a diff of elm against mc at the last mc-elm merge cset and store that in a base mc patch. Then add your changes on top of that and push to try. WinMerge works pretty well for this.

Comment 23

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to matthewdark from comment #21)
> Weird.  I can't get it to write anything to the log file.  I tried in
> %userprofile%\My Documents\note.txt and c:\note.txt and c:\
> 
> CMD reports:
> NSPR_LOG_FILE=c:\note.txt
> and I can edit it from notepad %NSPR_LOG_FILE%, so I'm pretty sure I have it
> set right.  So unless nightly isn't updating its env variables when I launch
> it from metro, I dunno what's up.
> 
> I've tried a couple locations and finally gave it c:\note.txt to make it as
> easy to parse as I could, and given Everyone full control of the file, so it
> should be able to access it.
> 
> It says the build date was 12-17 so it must be your latest version, maybe
> the patch didn't get patched? :)

There's one more environment variable to set
  Name: NSPR_LOG_MODULES
  Value: ALL

Sorry for leaving that out of the original instructions!

Comment 25

[matthewdark]

Attachment: Requested NSPR log

Got it to work!  I touch-clicked once to load a web site, then scrolled from the bottom up to the top, and then closed the browser by sliding in the from the top.

Comment 26

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Sorry for the delay; I've been out of town since last Friday, and I'll be out of town again starting tomorrow evening and ending January 7th.

Matthew: Thanks very much for submitting that log!

It looks like what's happening is that we're getting a PointerMoved event for a touchId that we've already received a PointerReleased event for.  This should be easy to fix; in fact, Jimm's original suggestion of simply ignoring PointerMoved events for touchIds that we don't expect to be processing is a simple and sufficient fix for this issue.

I'll rebase and provide an updated patch for review.

Comment 27

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v3

This should fix the issue.  The patch is untested since I'm not able to reproduce, but it's straightforward enough.  The patch also adds some logging that was missing from MetroInput.

Brian/Jim: I requested review from both of you since it's the holidays.  Whoever is able to get to the review first, please clear the other reviewer.

Comment 28

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Pushed to elm:
https://hg.mozilla.org/projects/elm/rev/03b57b4da1bd

Build containing the patch:
https://tbpl.mozilla.org/?tree=Elm&rev=03b57b4da1bd

Once the build completes, we'll need people who could reproduce the issue to test and verify that the issue is gone.  If the issue seems to be fixed, I'll mark this bug as [completed-elm]

Comment 29

[matthewdark]

Sorry, moved and I've been without internet for awhile.  I think I should be able to report back later tonight.

I'm not clear on how to get the installer out of that site, can I get a link?

Comment 30

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to matthewdark from comment #29)
> Sorry, moved and I've been without internet for awhile.  I think I should be
> able to report back later tonight.
> 
> I'm not clear on how to get the installer out of that site, can I get a link?

You can just use the latest elm build.  I'm not sure if there's a more appropriate link that will take you to the latest, but this ftp directory seems to have all the win32 release builds: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/tinderbox-builds/elm-win32

Comment 31

[matthewdark]

Got it going.  I'd say it's resolved.  Works as expected now for touch to click and touch+move to scroll.

Comment 32

[Matt Brubeck (:mbrubeck)]

Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.