Open
Bug 465257
Opened 16 years ago
Updated 2 years ago
Thunderbird should support multi-touch gestures on OS X
Categories
(Thunderbird :: Mail Window Front End, enhancement)
Tracking
(Not tracked)
NEW
People
(Reporter: susurrus, Unassigned)
References
(Blocks 2 open bugs)
Details
(Keywords: student-project, Whiteboard: [patchlove])
Attachments
(2 files, 7 obsolete files)
(deleted),
patch
|
standard8
:
review-
bwinton
:
ui-review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
standard8
:
review-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081116 Minefield/3.1b2pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081116 Shredder/3.0b1pre
Thunderbird on OS X should have gesture support. All gestures supported in the 3.1 nightlies are applicable:
pinch - zoom
2-finger rotate - tab switch
2-finger scroll - up/down
3-finger scroll - Top/bottom of page (would prefer if this was page up/down, but...)
See resolved bug 456520 for Firefox for more details.
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Comment 1•16 years ago
|
||
This sounds like something we'd want to support in TB (note that 2-finger scroll is already available).
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted-thunderbird3?
Version: unspecified → Trunk
Comment 2•16 years ago
|
||
Sounds like the right set of actions to gestures
Updated•16 years ago
|
Component: General → Mail Window Front End
Flags: wanted-thunderbird3? → wanted-thunderbird3+
QA Contact: general → front-end
Hardware: PowerPC → All
Comment 4•16 years ago
|
||
This bug is really hard to find. Lets give it a better summary and update its dependency list.
Depends on: 412486
Summary: Gesture support for OS X → Thunderbird should support multi-touch gestures on OS X
Comment 6•16 years ago
|
||
Conveniently, we didn't implement rotate for tab switch before Firefox removed it in bug 491925, so we don't need to follow them by removing it :)
Comment 7•15 years ago
|
||
I think the one thing left that makes the most sense is the 3-finger-scroll support for the message reader. We could easily hook this up to the home and end keys for the top and bottom of the message, respectively. However I think we'd ideally want a more preview like interaction where 3-finger-scroll is "more, faster scroll" and not exact paging.
Adding blake in case he has a free cycle and the inclination to investigate this.
Comment 8•15 years ago
|
||
At a minimum, whipping to the top and bottom of message content and message list views using three fingers up and down would be a huge win here.
Comment 9•15 years ago
|
||
This could be a good student project as it is really a matter of taking from firefox and making it available in Thunderbird. Here are some of the relevant references.
Steal from this:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#655
Paste gGestureSupport code somewhere into here:
http://mxr.mozilla.org/comm-central/source/mail/base/content/msgMail3PaneWindow.js#983
Add gGestureSupport.init() function to this OnLoadMessenger function:
http://mxr.mozilla.org/comm-central/source/mail/base/content/msgMail3PaneWindow.js#240
Keywords: student-project
Updated•15 years ago
|
Assignee: nobody → grimo
Status: NEW → ASSIGNED
Comment 10•15 years ago
|
||
awesome! hi andrew! feel free to email or post comments / questions into this bug.
Comment 11•15 years ago
|
||
Thanks Bryan, looking forward to joining on with this. I was advised to find you on irc and acquire some insight and direction into approaching this bug. I've reviewed the links to the code you've posted above and it seems that with the aid of development of these gestures in Firefox that it seems alot has been done to help this along.
What would you advise as first directions for going into this? I'm in the midst of setting up to build Thunderbird (or Shredder?) from source. I've got Firefox (or MinefieldDebug) running right now from source. I'll hold off from compiling and do an update for Thunderbird however when tinderbox stops burning.
Comment 12•15 years ago
|
||
Andrew, I'd suggest starting by getting the 3-finger scroll support working for home / end such that people can scroll all the way to the top of messages or the thread/message list.
Once we get to that point we can try to workout how we can make the home / end events extensible so that add-ons could provide a fast scroll or momentum scroll that replaces the home / end scroll.
I'm clarkbw on irc; feel free to ping me or anyone else.
Comment 13•15 years ago
|
||
Can I suggest that horizontal three-finger swipes flip backwards or forwards through messages in a thread? Besides keeping the action consistent with other programs in OS X, it would be very handy for long Email conversations or mailing list threads.
Comment 14•15 years ago
|
||
I'd like to present this first piece of code for comments from the community. It implements 3-finger swipe actions, scroll to the top/bottom and horizontal swipes scrolls through email threads. I'm interested in feedback to provide better implementation and any direction that you'd specifically like to see next.
Comment 15•15 years ago
|
||
Andrew could you produce a try build with your patch ?
Comment 16•15 years ago
|
||
I understand you need certain permissions for use of a try/server, is there anyone that could help me with that.
In the meantime, I have posted a link for a dmg package for mac. It was compiled and works on my MacBook Pro Intel with Leopard. I don't know how to control the name of the package, but the source code is from 3.7 nightly build source code.
http://matrix.senecac.on.ca/~agrimo/osd/thunderbird-3.0pre.en-US.mac.dmg
Comment 17•15 years ago
|
||
(In reply to comment #16)
> I understand you need certain permissions for use of a try/server, is there
> anyone that could help me with that.
Sorry I should have given you the link to https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer
> In the meantime, I have posted a link for a dmg package for mac. It was
> compiled and works on my MacBook Pro Intel with Leopard. I don't know how to
> control the name of the package, but the source code is from 3.7 nightly build
> source code.
> http://matrix.senecac.on.ca/~agrimo/osd/thunderbird-3.0pre.en-US.mac.dmg
Downloading it right now.
Comment 18•15 years ago
|
||
So for now it seems ok - just added calendar to my profile :-)
two finger scrolling works nicely. 3 finger scrolling only works in the message pane, I was unable to use it on the message list. Whilst trying to use it on the message list the message pane would scroll :-(
As rotate doesn't work in FF I probably need to change settings on my machine to test.
Comment 19•15 years ago
|
||
Comment on attachment 407922 [details] [diff] [review]
WIP - a first step solution enabling 3-finger swipes actions in 4 directions
Awesome work! I've been playing with it and I'm really liking it so far. I don't know how we can get the 3 finger swipe to apply to the message and folder list so I think we'll try to find some people who would know how to do that.
>diff -r 676a5cd80eab mail/base/content/msgMail3PaneWindow.js
>--- a/mail/base/content/msgMail3PaneWindow.js Sun Sep 27 23:59:18 2009 +0100
>+++ b/mail/base/content/msgMail3PaneWindow.js Thu Oct 22 21:23:45 2009 -0400
>@@ -42,16 +42,18 @@
> *
> * ***** END LICENSE BLOCK ***** */
>
> Components.utils.import("resource://gre/modules/folderUtils.jsm");
> Components.utils.import("resource://app/modules/activity/activityModules.js");
> Components.utils.import("resource://app/modules/jsTreeSelection.js");
> Components.utils.import("resource://app/modules/MailConsts.js");
>
>+<script type="application/javascript" src="chrome:global/content/globalOverlay.js" />
>+
And one quick code comment. You probably don't need this as I don't think this is doing anything. You can't use a script tag inside a JavaScript source file. :)
Comment 20•15 years ago
|
||
I pushed this into the try server builds, we should see something show in a couple hours: http://tinderbox.mozilla.org/showbuilds.cgi?tree=ThunderbirdTry
Comment 21•15 years ago
|
||
Looks like Standard8 got the try builds working
http://s3.mozillamessaging.com/build/try-server/2009-10-26_13:13-bugzilla@standard8.plus.com-multi-touch-mac/bugzilla@standard8.plus.com-multi-touch-mac-mail-try-mac.dmg
Comment 22•15 years ago
|
||
Here is a patch for review. Found some code that uses a synthesized mouse scroll event for swiping up/down and it now matches the pane that it is over to act on the scroll. This patch was made on rev:4ea7a625a6f9 and after I applied it I pulled the latest changesets and noticed that scrolling over a browser-like window (XPCNativeWrapper Window) now only pages mimicking pgUp/pgDown events. I noticed this change in behaviour for Firefox after a certain point aswell by using the "sendMouseScrollEvent( )" method.
There are no other multi-touch actions in place yet as I was trying to refine the swiping event targets, which now is happening for up/down. I found a way so far to isolate out which window pane is being targeted aswell in case this becomes pertinent for defined actions over a particular pane.
A sample of this snippet can be viewed at http://pastebin.mozilla.org/684959. I also have a downloadable Mac (intel) package that has this patch in place (and behaves with the pgUp/pgDown behaviour over messages), which it at: http://tinyurl.com/ykyphvd (you can preview the link before downloading).
Thoughts and direction... next behaviors to implement? code refinements? new suggestions?
Attachment #407922 -
Attachment is obsolete: true
Comment 23•15 years ago
|
||
I'm really looking forward to seeing this in Thunderbird!
For ideas for other actions, perhaps we could swipe (heh) some from:
http://www.danrodney.com/mac/multitouch.html
although it looks like you've already got most of them.
Later,
Blake.
Comment 24•15 years ago
|
||
(In reply to comment #22)
> A sample of this snippet can be viewed at http://pastebin.mozilla.org/684959.
> I also have a downloadable Mac (intel) package that has this patch in place
> (and behaves with the pgUp/pgDown behaviour over messages), which it at:
> http://tinyurl.com/ykyphvd (you can preview the link before downloading).
I'll try to test that once rc1 get's out.
Comment 25•15 years ago
|
||
Here's the latest and greatest... well, it took a lot of learning/experimenting.
I've set the latest patch up with preferences implemented and setup the commands in the messenger.xul file. I'm hoping this setup will allow for simple extension override of the events as was requested.
I explicitly setup swipe up/down to have mouse scrolling behaviour that pushes the page to the very top/bottom. It is isolating the various panes aswell. Also, I've put in the swipe left/right to manage selecting through the messages in the thread. Both of these are similar behaviours from the last patch.
You'll notice I've included a number of preferences and I've updated the code to match the latest source code from Firefox. With this, automatically pinch in/out will provide zoom in/out on the messages themselves. Also shift+pinch in/out will reset the zoom level.
With the latest firefox code they are implementing xulcommandevents and it works for everything except the mouse scroll function. This function requires the x/y coordinates of the property which seem to get lost in the use of this event mechanism, so for now, I've set it to use the older event approach.
I'm up for suggestions to improve the naming of preferences, or placement of code... or anything else. I'm intending on setting up a simple extension to override the behaviours (or shut them off) to establish that extensions will work with it. I'll provide a sample build of it then. Other next steps?
thanks.
Attachment #413596 -
Attachment is obsolete: true
Comment 26•15 years ago
|
||
Ok, here's my latest and well...
Here is a link to my packaged dmg installer for Thunberbird with my patch in it.
http://tinyurl.com/ykyphvd
And here is a link to an extension that I setup and it allows you to turn off/on the multi-touch events for swipes and pinches (essentially by removing and adding back in the preferences for them - and loads the preferences again on unload).
http://tinyurl.com/yb374ny
There is a problem with the extension though, for some reason if its installed normally, it won't work and has an overlay issue (noted from the error console). When you expand the files though and place a file named with the id and containing the path to the folder... it works fine. For the sake of sharing it earlier, I wanted to post it. I will continue to work on it though and will update the location of the current file and post about it here.
Comment 27•15 years ago
|
||
The link to the extension noted above no has the extension loading properly. So if you've applied the patch, or downloaded my trial Thunderbird build above, this extension will properly work.
Comment 28•15 years ago
|
||
I'm not very familiar with gestures. What do I need to use gestures with your patch? Is this only possible with the MacBook Air and Unibody MacBooks? (I have an iMac). Do I need an Apple Magic Mouse to use it?
Comment 29•15 years ago
|
||
(In reply to comment #28)
> I'm not very familiar with gestures. What do I need to use gestures with your
> patch? Is this only possible with the MacBook Air and Unibody MacBooks? (I have
> an iMac). Do I need an Apple Magic Mouse to use it?
yes you need a track pad. I think he magic mouse emulates gestures.
Comment 30•15 years ago
|
||
(In reply to comment #29)
> (In reply to comment #28)
> > I'm not very familiar with gestures. What do I need to use gestures with your
> > patch? Is this only possible with the MacBook Air and Unibody MacBooks? (I have
> > an iMac). Do I need an Apple Magic Mouse to use it?
>
> yes you need a track pad. I think he magic mouse emulates gestures.
I checked in at a local Apple Store, and the magic mouse only mimicks 2-finger scrolling gestures and doesn't support the multi-gesture actions that this patch implements. However, there are certain usb based tablets that also provide the same multi-touch gesture support as the latest Mac laptops. These special tablets would be 'the' solution for any user that doesn't have native multi-touch gesture support with their mac hardware... such as an iMac.
Comment 31•15 years ago
|
||
Ah, OK, thanks. Than it sadly seems I'm not able at the moment to test this patch...
Comment 32•15 years ago
|
||
(In reply to comment #31)
> Ah, OK, thanks. Than it sadly seems I'm not able at the moment to test this
> patch...
http://www.wacom.com/bamboo/bamboo_touch.php - This is the manufacturer's link, its $69 USD purchased directly through them and at that price its not outside of reasonable affordability. It may help multi-touch functionality be more popular for even desktop users with items like this making those features presented here, accessible to them aswell.
Comment 33•15 years ago
|
||
I haven't been able to test it because I'm running the nightly of 3.0 - not the alpha (daren't risk alpha's on my message base :-) but ....
Can I make a plea for matching Apple's recommendations for gestures, which as I understand it is that three-finger up and down are PAGE not top and bottom of screen.
This is important for those of us who prefer to read an email while its not moving, then page down to the next page.
At the very least, lets have a preference to allow for selecting page-up/down.
(Left and right three fingers for next and previous sound like a good idea - but best if its the next or previous i.e. equivalent of F and B shortcut keys rather than trying to go through a thread when threading isn't selected.
As an aside, its annoying that Firefox does the top-and-bottom of page thing with three-finger gestures, as it makes it much harder to page through a long document.
- Mitra
Comment 34•15 years ago
|
||
Hi, I was now able to test your patch with an Bamboo Touch on my iMac. And I like it. Sadly the Bamboo only supports one- and two finger gestures. I opened a mail with big pictures in the attachment, I've tried to zoom out and in into these pictures (and to rotate it). But this doesn't work. Is it also possible to support the zoom (and maybe rotate) gesture (e.g. for attachments)?
I've included your patch into my unofficial Thunderbird Trunk builds. :)
Reporter | ||
Comment 35•15 years ago
|
||
In regards to Mitra's comments in 33, I'd like to suggest that Thunderbird match Firefox's gestures. With the spacebar working as a page-down button and being conveniently right above the trackpad I see little reason for having a third way to trigger a page-down event. I also find myself using top/bottom of page gestures in firefox and would probably see the same thing in real-world usage in Thunderbird.
Comment 36•15 years ago
|
||
Since I have a Magic Mouse (two days now), I really love this patch. It's so cool and helpfull to swipe through your mails. :-)
So sadly that it seems this wouldn't make it into 3.1.
Comment 37•15 years ago
|
||
I wonder if someone could release it as an extension?
Comment 38•15 years ago
|
||
I've still got it on my radar. I have a few adjustments to it but must test them again as its been a while since my last patch. I'll need one or two weeks for my time to clear up before I can work on it again. Sorry for missing 3.1.
Comment 39•15 years ago
|
||
I use this patch in every of my own builds, but in the meantime its bitrottet. So I've unbitrottet it to current trunk. Its the same patch as above, only unbitrottet.
Comment 40•15 years ago
|
||
Thanks for getting the previous patch updated to the current code base.
Since my previous development I've updated my system to 10.6 (from OSX10.5) and I'm unable to get Thunderbird to build. Nonetheless, I had tested my changes in this patch using my previous 10.5 OS, but wasn't able to test against a current build. I haven't the time to get my build working but wanted to share the changes that I had in mind.
Changes in this patch are:
- I've migrated the process to completely adopt the xulcommandevent process.
- To do so I passed the aEvent as a property of the gGestureSupport object before the event is triggered and retrieved it using a gGestureSupport.MultiTouchScroll function call. This function subsequently calls BrowserScroll.
- The intention was to leave BrowserScroll in tact for scroll calls from events other than gGestureSupport events, but allowing gGestureSupport to call it directly for its own events.
- I also added a aPaging attribute to BrowserScroll which would allow for easy altering of scrolling behaviour to do paging instead of a scroll top/bottom. This allows for more flexibility of this functions use.
I'm not anticipating having time to trouble shoot my Thunderbird build in order to test this patch. It is intended as a proposed patch for someone else in the community to test and hopefully further develop to completion. I will continue to follow the bug to monitor comments, but hope that someone else can finish this work.
Comment 41•15 years ago
|
||
Thanks for uploading the patches, Andrew; that's great! If anyone is interested in running with these changes, I like Mitra's idea of packaging them up as an extension a lot, because that's likely to be significantly quicker than trying to get them landed in core.
Comment 42•14 years ago
|
||
(In reply to comment #40)
> It is intended as a proposed patch for someone else in the
> community to test
I've tested your latest patch in my TB build now for a few month with an Apple Magic Mouse and a Bamboo Touch and it workes just fine. I will now test it with my new MacBook Pro. If I find any issues, than I comment about it in this bug.
Attachment #449437 -
Attachment is obsolete: true
Comment 43•14 years ago
|
||
What is missing in this patch? I use your updated patch now since a few month in every of my builds and couldn't find any problems. So I wonder what is missing (because its not marked for review).
Comment 44•14 years ago
|
||
(In reply to comment #43)
> What is missing in this patch? I use your updated patch now since a few month
> in every of my builds and couldn't find any problems. So I wonder what is
> missing (because its not marked for review).
I can help with de-bitrotting but unfortunately that's all I can do since I don't have a Mac with multi-touch pad, just an evergreen 4 year old MBP..
Will attempt this in the next week or two..
Updated•14 years ago
|
Whiteboard: [patchlove]
Comment 45•14 years ago
|
||
(In reply to comment #44)
> (In reply to comment #43)
> > What is missing in this patch? I use your updated patch now since a few month
> > in every of my builds and couldn't find any problems. So I wonder what is
> > missing (because its not marked for review).
>
> I can help with de-bitrotting but unfortunately that's all I can do since I
> don't have a Mac with multi-touch pad, just an evergreen 4 year old MBP..
>
> Will attempt this in the next week or two..
It's very easy to unbitrot, because only all-thunderbird.js is bitrotted. If you are to bussy, I also can do it.
Comment 46•14 years ago
|
||
I'm going to kick myself for saying this, but…
I have a fancy new-ish MBP with the multi-touch gestures, so I could test the patch, if someone pointed me at a try-server build.
Later,
Blake.
Comment 47•14 years ago
|
||
(In reply to comment #46)
> I have a fancy new-ish MBP with the multi-touch gestures, so I could test the
> patch, if someone pointed me at a try-server build.
>
> Later,
> Blake.
There are links to older builds with this patch in Comment 26. If you want I can also point you to my own build which also include the latest patch (this is a 3.4a build).
Comment 48•14 years ago
|
||
(What I really want is a try-server build with the de-bit-rotted patch, so that I can see that it passes all the tests on all the platforms, as well as give it a test run for myself, so I'm afraid your build only gives me half of what I'm looking for… :) Thanks, though.)
Comment 49•14 years ago
|
||
I'll push to try as soon as de-birotted patch appears.
Comment 50•14 years ago
|
||
(In reply to comment #49)
> I'll push to try as soon as de-birotted patch appears.
This should be the smallest problem. :-)
Comment 51•14 years ago
|
||
push to try http://hg.mozilla.org/try-comm-central/rev/2f2a942ffc10, but I'm not getting the emails. Will update with email content and link when I get them.
Comment 52•14 years ago
|
||
Comment 53•14 years ago
|
||
mac failed. Repushed.
Comment 54•14 years ago
|
||
Comment 55•14 years ago
|
||
Comment on attachment 528699 [details] [diff] [review]
Patch that applies to latest comm-central
Review of attachment 528699 [details] [diff] [review]:
Okay, I've been testing this out this morning, and I'm really liking it. :)
I do kind of miss the rotate-to-switch-tabs, but even without that, this seems like a win.
ui-r=me!
Thanks,
Blake.
Attachment #528699 -
Flags: ui-review+
Comment 56•14 years ago
|
||
(In reply to comment #55)
> I do kind of miss the rotate-to-switch-tabs, but even without that, this seems
> like a win.
>
> ui-r=me!
>
> Thanks,
> Blake.
I've marked it for review than. :-)
Attachment #528699 -
Flags: review?(bugmail)
Comment 57•14 years ago
|
||
Comment on attachment 528699 [details] [diff] [review]
Patch that applies to latest comm-central
Review of attachment 528699 [details] [diff] [review]:
Deferring review to Standard8; I lack a device capable of generating multitouch events.
Attachment #528699 -
Flags: review?(bugmail) → review?(mbanner)
Updated•14 years ago
|
Attachment #417847 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #449467 -
Attachment is obsolete: true
Comment 58•14 years ago
|
||
I'll be taking a detailed look at this as soon as I can after the 10th May. I'd like to try it out in detail for a few days, but I'm busy getting 3.3 ready for branching (if it gets r+ it'll go in the next release which is scheduled for 6-8 weeks after 3.3).
Comment 59•14 years ago
|
||
Comment on attachment 528699 [details] [diff] [review]
Patch that applies to latest comm-central
Review of attachment 528699 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, I've given this a run through. As an initial version it looks good, but there's some tweaks where I think we can do things better.
Additionally, this doesn't currently work in anything other than the three-pane message window. I think we can do slightly better than that, I'll add pointers on the way through the comments.
::: mail/app/profile/all-thunderbird.js
@@ +686,5 @@
> +// simple gestures support
> +pref("browser.gesture.swipe.left", "cmd_previousMsgMultiTouch");
> +pref("browser.gesture.swipe.right", "cmd_nextMsgMultiTouch");
> +pref("browser.gesture.swipe.up", "cmd_mouseScrollUp");
> +pref("browser.gesture.swipe.down", "cmd_mouseScrollDown");
I think these should be made to call cmd_scrollTop and cmd_scrollBottom. These use the built-in functions, and then we don't need to spin our own (especially as that is doing things which I believe may be wrong anyway).
@@ +695,5 @@
> +pref("browser.gesture.pinch.latched", false);
> +pref("browser.gesture.pinch.threshold", 25);
> +#endif
> +pref("browser.gesture.pinch.out", "cmd_fullZoomEnlarge");
> +pref("browser.gesture.pinch.in", "cmd_fullZoomReduce");
Firefox disabled browser.gesture.pinch.out(.shift) and browser.gesture.pinch.in(.shift) in bug 613909, I think we should probably have these disabled as well for the same reasons as mentioned there.
::: mail/base/content/messenger.xul
@@ +152,5 @@
> #endif
> + <command id="cmd_previousMsgMultiTouch" oncommand="goDoCommand('cmd_previousMsg')" disabled="false"/>
> + <command id="cmd_nextMsgMultiTouch" oncommand="goDoCommand('cmd_nextMsg')" disabled="false"/>
> + <command id="cmd_mouseScrollUp" oncommand="gGestureSupport.MultiTouchScroll(false, false);" disabled="false"/>
> + <command id="cmd_mouseScrollDown" oncommand="gGestureSupport.MultiTouchScroll(false, true);" disabled="false"/>
If we use cmd_scrollTop and cmd_scrollBottom like I mentioned previously, then we can drop the commands for cmd_mouseScrollUp and cmd_mouseScrollDown completely.
::: mail/base/content/msgMail3PaneWindow.js
@@ +399,5 @@
>
> window.addEventListener("AppCommand", HandleAppCommandEvent, true);
> +
> + // setup simple gestures support
> + gGestureSupport.init(true);
There's not a code to gGestureSupport.init(false); anywhere, which means the listeners it sets up never get removed.
I would suggest moving this call to InitMsgWindow in mailWindow.js and the unintialisation into OnMailWindowUnload.
For the compose window, you probably want to put calls into ComposeLoad and ComposeUnload.
For the address book window, OnLoadAddressBook and OnUnloadAddressBook.
@@ +1170,5 @@
> +// API is undocumented and was reverse-engineered.) Until support is
> +// implemented in the event dispatcher to keep these events as
> +// chrome-only, we must listen for the simple gesture events during
> +// the capturing phase and call stopPropagation on every event.
> +let gGestureSupport = {
iirc all of the four window types include utilityOverlay.js, so lets move this to that file, so that it can be used in all of them.
@@ +1336,5 @@
> + let command;
> + try {
> + command = this._getPref(aGesture.concat(subCombo).join("."));
> + } catch (e) {}
> + if (command){
This should be the same as the Firefox version, i.e.
if (!command)
continue;
@@ +1348,5 @@
> + node.dispatchEvent(cmdEvent);
> + }
> + // No event is called if it is defined as disabled.
> + // Previously it was used to call commands using an aEvent parameter.
> + //} else {
This commenting out can be removed.
@@ +1402,5 @@
> + // Determine what type of data to load based on default value's type
> + let type = typeof aDef;
> + let getFunc = "get" + (type == "boolean" ? "Bool" :
> + type == "number" ? "Int" : "Char") + "Pref";
> + return gPrefBranch[getFunc](branch + aPref);
This should use Services.prefs.
@@ +1420,5 @@
> + * determine paging behaviour.
> + * @param aBottom
> + * True to scroll to the bottom; otherwise, go up
> + */
> + MultiTouchScroll: function GS_scroll(aPaging,aBottom){
See above, if we use the built-in functions then we don't need this one or the BrowserScroll function.
Attachment #528699 -
Flags: review?(mbanner) → review-
Comment 60•14 years ago
|
||
In this patch I fixed the easier review comments (and the patch still works). I now will try to fix the "harder" parts. Not yet fixed is:
::: mail/base/content/msgMail3PaneWindow.js
@@ +399,5 @@
>
> window.addEventListener("AppCommand", HandleAppCommandEvent, true);
> +
> + // setup simple gestures support
> + gGestureSupport.init(true);
There's not a code to gGestureSupport.init(false); anywhere, which means the listeners it sets up never get removed.
I would suggest moving this call to InitMsgWindow in mailWindow.js and the unintialisation into OnMailWindowUnload.
For the compose window, you probably want to put calls into ComposeLoad and ComposeUnload.
For the address book window, OnLoadAddressBook and OnUnloadAddressBook.
@@ +1170,5 @@
> +// API is undocumented and was reverse-engineered.) Until support is
> +// implemented in the event dispatcher to keep these events as
> +// chrome-only, we must listen for the simple gesture events during
> +// the capturing phase and call stopPropagation on every event.
> +let gGestureSupport = {
iirc all of the four window types include utilityOverlay.js, so lets move this to that file, so that it can be used in all of them.
@@ +1402,5 @@
> + // Determine what type of data to load based on default value's type
> + let type = typeof aDef;
> + let getFunc = "get" + (type == "boolean" ? "Bool" :
> + type == "number" ? "Int" : "Char") + "Pref";
> + return gPrefBranch[getFunc](branch + aPref);
This should use Services.prefs.
Comment 61•14 years ago
|
||
This patch includes now all review comments, except for one which I don't know how to fix. But it seems something is wrong with the patch, because gestures doesn't work in the addressbook. But I don't know why. Maybe you have an idea? Apart from that, the patch works fine for me.
The only not fixed review comment (because don't know how), is:
@@ +1402,5 @@
> + // Determine what type of data to load based on default value's type
> + let type = typeof aDef;
> + let getFunc = "get" + (type == "boolean" ? "Bool" :
> + type == "number" ? "Int" : "Char") + "Pref";
> + return gPrefBranch[getFunc](branch + aPref);
This should use Services.prefs.
Attachment #534237 -
Attachment is obsolete: true
Comment 62•14 years ago
|
||
With a hint from rsx11m I found what was missing to fix the last comment. But if I use gPrefService instead of gPrefBranch (like in browser.js), than it doesn't work anymore. So I didn't change this. But I've unbittroted it and fixed one additional thing.
Attachment #534242 -
Attachment is obsolete: true
Attachment #536695 -
Flags: review?
Comment 63•14 years ago
|
||
Comment on attachment 536695 [details] [diff] [review]
patch
Helping to forward the review to Standard8, who was a reviewer for the some of the past versions of the patch.
Attachment #536695 -
Flags: review? → review?(mbanner)
Comment 64•13 years ago
|
||
So that I don't forget about this again, I think I've still got some concerns about this patch. When it is applied, it seems to stop the three-finger swipe working in the list views of the application, at the expense of having it work in the browser part.
I'm not quite sure what is causing this to happen it seems like the underlying code supports three-finger swipe in the list views but not in the browser/message pane.
Comment 65•13 years ago
|
||
Sadly I have also no idea what could cause this. :-( Hope there is a solution for this!
Comment 66•13 years ago
|
||
Comment on attachment 536695 [details] [diff] [review]
patch
This needs more work - see my comment 64 for the issues. I don't think we can do this at the expense of loosing three-finger scrolling in the folder/message list views as well.
Attachment #536695 -
Flags: review?(mbanner) → review-
Comment 67•13 years ago
|
||
I heard rumors that under 10.7 two-finger swipe is also supported. Today I was thinking it would be sweet if this could be use to expand conversation threads in the thread pane. What do you think?
Comment 68•13 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #67)
> I heard rumors that under 10.7 two-finger swipe is also supported.
Do you mean like Bug 668953?
Comment 69•11 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #64)
> So that I don't forget about this again, I think I've still got some
> concerns about this patch. When it is applied, it seems to stop the
> three-finger swipe working in the list views of the application, at the
> expense of having it work in the browser part.
>
> I'm not quite sure what is causing this to happen it seems like the
> underlying code supports three-finger swipe in the list views but not in the
> browser/message pane.
Comment 70•11 years ago
|
||
The sad thing is, this patch doesn't work at all anymore. And since this patch was written the touch gesture code in Core has changed a lot.
Updated•9 years ago
|
Assignee: u358732 → nobody
Status: ASSIGNED → NEW
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•