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)

Product:
Thunderbird
Component:
Mail Window Front End
Platform:
All
macOS
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: susurrus, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: student-project, Whiteboard: [patchlove])

Attachments

(2 files, 7 obsolete files)

Patch that applies to latest comm-central
15.35 KB, patch
standard8
: review-
bwinton
: ui-review+
Details | Diff | Splinter Review
patch
16.20 KB, 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.
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
Sounds like the right set of actions to gestures
Component: General → Mail Window Front End
Flags: wanted-thunderbird3? → wanted-thunderbird3+
QA Contact: general → front-end
Hardware: PowerPC → All
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
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 :)
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.
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.
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
Assignee: nobody → grimo
Status: NEW → ASSIGNED
awesome! hi andrew!  feel free to email or post comments / questions into this bug.
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.
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.
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.
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.
Andrew could you produce a try build with your patch ?
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
(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.
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 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. :)
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
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
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.
(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.
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
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.
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.
Blocks: TB2SM
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?
(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.
(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.
Ah, OK, thanks. Than it sadly seems I'm not able at the moment to test this patch...
(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.
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
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. :)
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.
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.
I wonder if someone could release it as an extension?
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.
Attached patch Unbitrottet above patch (obsolete) — — Splinter Review 
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.
Attached patch untested_but_slight_update (obsolete) — — Splinter Review 
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.
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.
(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
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).
(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..
Whiteboard: [patchlove]
(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.
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.
(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).
(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.)
I'll push to try as soon as de-birotted patch appears.
(In reply to comment #49)
> I'll push to try as soon as de-birotted patch appears.
This should be the smallest problem. :-)
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.
mac failed. Repushed.
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+
(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 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)
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 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-
Attached patch partialy fixed comments (obsolete) — — Splinter Review 
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.
Attached patch One comment left (obsolete) — — Splinter Review 
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
Attached patch patch — — Splinter Review 
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 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)
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.
Sadly I have also no idea what could cause this. :-( Hope there is a solution for this!
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-
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?
(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?
Blocks: tb-mac
(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.
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.
Assignee: u358732 → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: