On newer MacBooks with the multi-touch trackpad, there's a setting (on by default) to make two-finger scrolling scroll with momentum. I'm not sure if we have special code to handle this, or if we get it for free from the system. If you two-finger scroll, the scroll continues for a bit after you remove your fingers (the momentum bit). However, if you press the Ctrl key while this scrolling continues, we translate it into page zoom, even if your fingers are no longer touching the trackpad. I find myself accidentally doing this all the time because I'll scroll down a page, then hit Ctrl+PgUp/PgDn to switch tabs.

I'm using a 32-bit trunk nightly on OS X 10.6.3. I just got this MacBook Pro about a week ago, it's one of the new Core i7 models.

The momentum scrolling comes for free from the system - it's just repeatedly sending scroll events. http://stackoverflow.com/questions/1849023/how-does-the-momentum-inertial-scroll-work-with-the-magic-mouse-on-nsscrollview#answer-1849870 says we discern normal scroll NSEvents from automatic repeated NSEvents by checking the scrollPhase field. Unfortunately I haven't found any documentation of that, and I don't have a device to test it.

Damon has one of these - maybe he can confirm for us?

Markus: if you can put together a sample xcode project or something, I'd be happy to build and run it and give you the results.

I can, maybe next week.

Yep. Confirmed.

Joe: this is really visible and really annoying on newer MBPs. If you haven't seen it, go find someone that has one and try it out. I really think we should block on it. I hit it almost constantly while using my MBP, althought my workflow could be unique.

I have one of these MacBooks too! And a Magic Mouse which does the same thing. But I don't run into this at all. However, I understand what you're saying, so let's say we'll block on this for now, since we always have the possibility to unblock on it later.

Here's a test app that dumps the _scrollPhase and _continuousScroll fields to the console. Can somebody with such a device run this for me, please? 1. Save as momentum.m 2. Go to the directory with Terminal 3. Run gcc -framework Cocoa -lobjc momentum.m -o momentum && ./momentum A gray window will open in the lower left corner of the screen. 4. Scroll over that window. Stuff will be dumped to the console. 5. Terminate with Ctrl+C. 6. Paste the output here.

Interesting. Can you add another log where you scroll down fast, and then while momentum scroll is still scrolling down, scroll up again?

as requested

Does this work?

It does! But it also causes momentum scrolling to stop while ctrl is pressed.

How about this?

It works!

Comment on attachment 459132 [details] [diff] [review] v2 >diff --git a/content/events/src/nsEventStateManager.cpp b/content/events/src/nsEventStateManager.cpp >--- a/content/events/src/nsEventStateManager.cpp >+++ b/content/events/src/nsEventStateManager.cpp >@@ -2939,26 +2939,30 @@ nsEventStateManager::PostHandleEvent(nsP > nsContentUtils::GetBoolPref(sysNumLinesKey.get()); > > if (useSysNumLines) { > if (msEvent->scrollFlags & nsMouseScrollEvent::kIsFullPage) > action = MOUSE_SCROLL_PAGE; > } > > if (aEvent->message == NS_MOUSE_PIXEL_SCROLL) { >- if (action == MOUSE_SCROLL_N_LINES) { >+ if (action == MOUSE_SCROLL_N_LINES || >+ (msEvent->scrollFlags & nsMouseScrollEvent::kIsMomentum)) { > action = MOUSE_SCROLL_PIXELS; > } else { > // Do not scroll pixels when zooming > action = -1; > } > } else if (msEvent->scrollFlags & nsMouseScrollEvent::kHasPixels) { > if (action == MOUSE_SCROLL_N_LINES) { > // We shouldn't scroll lines when a pixel scroll event will follow. > action = -1; >+ } else if (msEvent->scrollFlags & nsMouseScrollEvent::kIsMomentum) { >+ // Don't do history scrolling or zooming for momentum scrolls. >+ action = -1; > } Why not combine 'if' and 'else if'? They both set action to -1 I don't know too much about OSX specific bits.

Backed out because the test fails on Windows. Not sure why yet. http://hg.mozilla.org/mozilla-central/rev/bd4713e47e5e http://hg.mozilla.org/mozilla-central/rev/8fbcb029ca0a http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1280759437.1280762835.24943.gz#err0 > 36128 INFO TEST-PASS | /tests/content/events/test/test_bug574663.html | Normal scrolling shouldn't change zoom - 1 should equal 1 > 36129 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug574663.html | Normal scrolling should scroll - didn't expect 0, but got it > 36130 INFO TEST-PASS | /tests/content/events/test/test_bug574663.html | Normal scrolling shouldn't change zoom, even after releasing the touchpad - 1 should equal 1 > 36131 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug574663.html | Normal scrolling should scroll, even after releasing the touchpad - didn't expect 0, but got it

Have you had a chance to look into why the test failed? Do you have a Windows environment to test in?

I've looked into it now and I think I've found out what was wrong: I was using SimpleTest.executeSoon(f) instead of setTimeout(f, 0) when waiting for the scroll position to change. Mouse scroll events start a zero-interval timer (in nsGfxScrollFrameInner::ScrollTo) which updates the scroll position, and my check function was firing before the scroll position timer had a chance to fire. The test seems to pass reliably now that I'm using setTimeout(f, 0). I've pushed the new patch to the tryserver (c80be47c7b8e); let's see what it says.

(In reply to comment #24) > Mouse scroll events start a zero-interval timer (in > nsGfxScrollFrameInner::ScrollTo) which updates the scroll position, and my > check function was firing before the scroll position timer had a chance to > fire. That's pretty odd. I would expect that since bug 512645 landed, |setTimeout(f1, 0); executeSoon(f2);| would result in f1 always being called first. setTimeout uses nsITimers under the hood, so that shouldn't be any different than your situation here. Worth adding a comment to your test ("can't depend on ordering of timers and executeSoon") if that really isn't the case. I'm kind of surprised it wouldn't have affected us elsewhere already - maybe it's the cause of some of our orange!

http://hg.mozilla.org/mozilla-central/rev/0eac6a562867 I'll file a new bug for the setTimeout/executeSoon ordering issue.

This bug still occurs in the latest Firefox version.