Closed Bug 698761 Opened 9 years ago Closed 8 years ago
.7] Add support for Chrome-style swipe animation
3.10 KB, text/plain
39.78 KB, text/plain
40.83 KB, text/plain
23.37 KB, patch
|Details | Diff | Splinter Review|
This bug is spun off from bug 668953 (particularly from bug 668953 comment #172). We need visual cues for two-finger horizontal swipes. Otherwise it's too easy to perform them accidentally, which can lead to loss of data in a form which has been filled in but not yet submitted. Markus Stange has been working on implementing Safari-style swipe animation at bug 678392. But the work is complex, and hasn't yet been finished. Chrome Canary implements a different kind of animation for two-finger horizontal swipes -- an overlay that follows the swipe until it's finished. This is much easier to implement, and so should be much easier to bring to a fully polished state. For now at least, I'm not saying that this is what we should do. But I've already created an implementation that I think is close to release quality. It can serve as a backup if we're not able to finish the work on bug 678392 in time.
Here's a patch that (basically) just copies the code that implements Chromium's "traveling arrow". Though I did change the arrows to left and right pointing triangles, because they more closely resemble the symbols we already use for the Back and Forward buttons. Here's a tryserver build made with my patch: http://firstname.lastname@example.org/try-macosx64/firefox-10.0a1.en-US.mac.dmg Please try it out! And comment here about any problems you find. I already know my "traveling triangles" are kind of ugly. For one thing they're different sizes. So it's probably better to use graphics instead of Unicode font characters -- ideally the same graphics we use for the Back and Forward buttons. I'd appreciate someone's help locating those graphics, and advice on how to use them (for example on how to lay them out, and to size the window that contains them).
Assignee: nobody → smichaud
Status: NEW → ASSIGNED
(In reply to Steven Michaud from comment #1) > Here's a tryserver build made with my patch: > http://email@example.com- > 53a8a65f42d2/try-macosx64/firefox-10.0a1.en-US.mac.dmg > > Please try it out! And comment here about any problems you find. Looks nice! But it crashes when swipe back and forth is done fast without waiting page to load: https://crash-stats.mozilla.com/report/index/bp-23fd0f5c-a067-47fc-aac8-959ae2111101
> Looks nice! But it crashes when swipe back and forth is done fast > without waiting page to load: Please give URLs of pages you crash on, when swiping back and forth between them. Do you also crash without my patch (on, say, today's mozilla-central nightly)? I'll work on translating the symbols in your Breakpad crash stack.
I could not get mozilla-central's nightly build to crash but your build crashes easily. And your build does not crash when cmd+left/right arrow navigation is used. I tried couple of sites, at least pages with rich content does crash. E.g: 1. Open http://www.theonion.com/ 2. Click “Glenn Beck Appears In Revealing Documentary About Brooke Alvarez's Childhood As Russian Cosmonaut” 3. Swipe back and immediately after that to forward. 4. Continue until Firefox crashes, usually few times is enough for me.
I don't really like this solution even apart from the current design of the arrows. But it's definitely much better than the current behavior. So I hope that this solution will not be needed for too long and the real animation can be landed rather sooner than later. Of course I know that it's not trivial, so I'll try to keep my hopes modest. :)
Here are the symbol translations for Simo's Breakpad crash stack. I don't fully trust it -- parts of it are truly weird. But the top part puts the blame pretty squarely on my patch, at least as the crashes' trigger. But the true cause may turn out to be an Apple bug -- possibly a bug in Apple's block support code. I used atos -o [modulename] -arch x86_64 [address] to do the translations. I'll work on reproducing the crashes myself, and getting a gdb crash stack, which is likely to be much more reliable.
I've managed to get a gdb stack trace of the crash, following Simo's STR from comment #4. The top part is the same as the bp stack, but the rest of it makes a *lot* more sense.
Attachment #571092 - Attachment is obsolete: true
Using the tryserver build, the arrows appear to be the opposite direction of my swipe. I'm using a magic mouse on 10.7.2. Is this intentional? This is a good stop gap solution to the full animation that is being worked on right now.
> Using the tryserver build, the arrows appear to be the opposite > direction of my swipe. I'm using a magic mouse on 10.7.2. Is this > intentional? No ... or at least I don't think so. Do the arrows move in the "normal" direction when you do a two-finger swipe on the trackpad? Does the same reversal of direction happen with Google Canary? I really need to get a magic mouse to test with ...
Testing out the build, Firefox just crashed. Here's the link to the crash report -- https://crash-stats.mozilla.com/report/index/bp-2a008ccf-356e-41e5-b8bc-f8d0a2111101 On the trackpad, I see the same behavior with the arrows that I see on the magic mouse. However, looking at the Canary build on Chrome, it's the same behavior. Thinking about this a bit more, the arrow animation is less intuitive vs. the full page animation shown in Safari. I retract my previous statement in comment 9 and the animation looks correct (even though it's still the opposite direction of my swipe).
iirc the arrow represents the direction of the page and not the direction your hand moves. On Windows at least, it is more intuitive with up / down swipes where the page is moving down towards the bottom of the page with the arrow pointing down with the swipe being upward.
Here's a patch that fixes the crashes. Turns out that Apple's tracking handler for -[NSEvent trackSwipeEventWithOptions:...] has unexpected behavior -- it can be called more than once with isComplete == true. A tryserver build should be available in a few hours.
Attachment #571029 - Attachment is obsolete: true
(In reply to comment #12) Does Windows have swipe tracking animation? Is it only Windows 7? I currently can't run Windows 7 (or Vista). But at some point I'll spend the time needed to install it into a VM.
Here's a tryserver build made with my patch from comment #13: http://firstname.lastname@example.org/try-macosx-debug/firefox-10.0a1.en-US.mac.dmg Please try it out. The crashes should be fixed.
Just tried this build and swipe is broken for me. Since back swipe doesn't work, there is no animation. Am I missing something?
Oops, I pasted in the wrong link. Here's the correct one: http://email@example.com/try-macosx64/firefox-10.0a1.en-US.mac.dmg Sorry for the confusion.
Works now with this build. So far it seems to work well and the back/forward swipes and snappy and the animation helps with the overall browsing experience. I'll let you know if I run into any issues with this new build.
I just got a different crash with the new build: bp-32660649-4549-4c37-bb7a-d4f9e2111102 The top two lines translate to: ___trackSwipeWithScrollEvent_block_invoke_1 (in AppKit) + 2965 __-[ChildView maybeTrackScrollEventAsSwipe:scrollOverflow:]_block_invoke_1 (in XUL) + 410 So my work isn't yet done :-(
Oops, got the top two lines reversed. They should be: __-[ChildView maybeTrackScrollEventAsSwipe:scrollOverflow:]_block_invoke_1 (in XUL) + 410 ___trackSwipeWithScrollEvent_block_invoke_1 (in AppKit) + 2965
I'll be working on this.
> I'll be working on this. The problem was very simple, and I've fixed it -- we need to null check mGeckoChild in -[NSEvent trackSwipeEventWithOptions:...]'s tracking handler. The bug is in existing code (not my patch). My patch just uncovered it. But I can't yet start a tryserver build, because there's a terrible bug in current trunk code, and any build I did now would crash on startup (see bug 694896). So I'll post my patch and do my tryserver build tomorrow (by which time the jemalloc bug should have been fixed).
Here's a version of my patch that fixes the 2nd crash. And here's a tryserver build made from it: http://firstname.lastname@example.org/try-macosx64/firefox-10.0a1.en-US.mac.dmg
Attachment #571376 - Attachment is obsolete: true
A new tryserver build will follow in a few hours.
Attachment #572067 - Attachment is obsolete: true
Hi Steven, cool you're working on this. I'd recommend that you trigger the navigation on `phase == NSEventPhaseEnded` instead of on `isComplete` – the former is sent earlier once it's clear that the user will complete the gesture. Say the user throws a swipe across the window and stops after 50%, then the swipe will complete, and you'll get phase == NSEventPhaseEnded as soon as the user lets go, but the value will continue to animate to 1.0 and only then send isComplete. Nico
Stephen Pohl is almost finished with bug 678392 so I am going to dupe this to that bug.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 678392
You need to log in before you can comment on or make changes to this bug.