457546 - add Quick look support to Thunderbird (preview attachments, QuickLook)

Status: NEW

(Thunderbird :: Mail Window Front End, enhancement)

Description

[Przemyslaw Bialik]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080928030741 Shredder/3.0b1pre

"Using Quick Look in Leopard, you can view the contents of a file without even opening it. Flip through multipage documents. Watch full-screen video. See entire Keynote presentations. With a single click"

Add Quick look support to Thunderbird and Apple Mail users would have 1 reason less not to move to Thunderbird ;)

Reproducible: Always

Steps to Reproduce:
1. Open e-mail with attachment (e.g. .pdf)
2. Click on attachment and press space bar
Actual Results:  
Nothing happens

Expected Results:  
Attachment is opened in Quick look

Comment 1

[Przemyslaw Bialik]

Related -  Bug 405358

Comment 2

[Phil Ringnalda (:philor)]

Certainly not blocking, given the current state of the public API (the only thing that's not private is "QLGetACheesyStaticThumbnailImage"), though it'd be interesting to see a patch for even that part.

Comment 4

[Nomis101]

I would also highly like it if I could preview attachments in Thunderbird with QuickLook. This would be a great enhancement. Maybe we can use a similar patch for that like in Bug 405358 (?).

Comment 6

[David Humphrey (:humph)]

I need to finish up the growl and spotlight stuff, but I'll look into this when that's done, if no one gets to it first.

Comment 7

[David Humphrey (:humph)]

Can I get some wisdom on how to attack this?  Specifically, I'm thinking to make a new component to do something similar to what's happening in the patches for bug 405358, but I'd like to know where this should live?  Does it belong in toolkit or somewhere else, and what to name it?

Comment 8

[Thomas D. (:thomas8)]

Adding " (preview attachments, QuickLook)" to the summary would would improve the retrievability of this bug in related searches.

Comment 9

[Mike Arvela]

Most of the time (for myself, at least) it is enough to be able to quickly preview attachments instead of first having to save them somewhere on the disk. Providing a native Quicklook preview in Thunderbird would work fantastically in this manner and I myself would consider it a killer feature.

Comment 10

[Nomis101]

Is there any idea how to do this? Or (maybe) a first patch in process? :-)

Comment 11

[Steve McMillen]

This would be a very well received feature.  Hope you or someone can find the time to implement!

p.s. you can double click to go to the full editor today. Big downside to that is it leaves a file in the 'downloads' folder.  Goal of this feature should be to clean up after itself and not leave the hard drive littered with garbage.

Comment 12

[Eugene Savitsky]

Could this bug set for a wider range of OS? In Outlook 2007 there is also such functionality (I saw it in Outlook working with Exchange).

Comment 13

[Magnus Melin [:mkmelin]]

That would be another bug as the implementation would be completely different.

Comment 14

[Nomis101]

Aside from attachments, I think it would also be nice if the Quicklook function would also be work to preview mails (additionally to the message pane view). I do so everyday in the Finder to preview files and this is very useful.

Comment 15

[To do]

I went around the problem by creating an automator script which I saved as an app. When I want to use quicklook, I choose the automator script I created. It would be nice to use an applescript or similar instead and make it a standard option in the attachment dialog box.

Comment 16

[David Humphrey (:humph)]

I'm not going to get to this any time soon, throwing back into the pot.

Comment 17

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #16)
> I'm not going to get to this any time soon, throwing back into the pot.

Might be a good student-project, though the difficulty might be slightly higher.

Comment 18

[David :Bienvenu]

Would this be a candidate project for Google summer of code?

Comment 19

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #18)
> Would this be a candidate project for Google summer of code?

Possible, but it would require a mentor.

Comment 20

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Daniel's doing this for GSoC 2010.

http://socghop.appspot.com/gsoc/student_project/show/google/gsoc2010/mozilla/t127230761176

Comment 21

[Nomis101]

In 405358 (#22) it is mentioned that SL add support for using QuickLook inside your application. Maybe this can help.

Comment 22

[Daniel]

I'd like to announce that v1.0 of the QL-Enabler extension is available for download at https://www.comp.nus.edu.sg/~nguyenld/QL-Enabler/QL-Enabler-testing-suite-v1.0.zip.

Please go through readme.txt :)

And, the md5 checksum is 31D092B859EE580D9F54BC915F2A061C.

Comment 23

[Nomis101]

Nice to see your progress in this, I intrigued follow your blog posts. :-) I've tested your extension today. But your provided Shredder doesn't start for me. I get everytime I try to start your build:

Dyld Error Message:
  Library not loaded: @executable_path/libmozjs.dylib
  Referenced from: /Volumes/Macintosh HD 2/Downloads/QL-Enabler-testing-suite-v1.0/Shredder.app/Contents/MacOS/thunderbird-bin
  Reason: image not found

Then I've looked inside your Shredder.app and the dylibs are mostly only aliases. 

So I've installed it into my own 10.6 Shredder. Installing worked fine, I've installed it via drag and drop of the extension to Shredders extensions window. I see the new context menu "QuickLook These..." and "QuickLook All...". But it doesn't preview my attachments. I've also tried the other methods you described in your readme. I've tested with JPEG, PDF and TXT. There is nothing in the error console.
I'm now unsure, did I something wrong?? If it works for you, than normally it also should work for me, I think?

Comment 24

[Daniel]

Hi Nomis,

I'm trying to find an explanation for your issue. I'm testing my release build on another laptop to see if it works. Btw, the link at the bottom is the .mozconfig I'm using to compile the release build. Maybe there's something wrong with it? The dylibs should not be aliases.

http://pastebin.mozilla.org/742786

Comment 25

[Nomis101]

Hi Daniel,
could it be that you've only forgotten to build the installer (dmg)?
make -C obj-x86_64-apple-darwin10.4.0/mail/installer
https://developer.mozilla.org/en/Build_and_Install

Is "qlxpcom" your extension?

Comment 26

[Daniel]

I've fixed the issue Nomis101 is facing. The extension is available for download at https://www.comp.nus.edu.sg/~nguyenld/QL-Enabler/QL-Enabler-testing-suite-v1.0.zip.

You can also find additional QuickLook plugins on http://www.qlplugins.com/.

Comment 28

[Nomis101]

As you maybe have read in Daniels blog (grbmozilla.wordpress.com), I've build a Shredder try version that includes Daniels Quicklook extension, so it is easy to test Daniels awesome work. Because this build was old and Thunderbird is now on libxul, I've tried to build a new version of Shredder with Daniels extension (which was different now). If you want to download this build to test Daniels Quicklook extension, you can find these new builds there:
• English (en-US): http://tinyurl.com/3amuy43
• Deutsch (de): http://tinyurl.com/352eu3g

Shortcut for all attachments is ctrl+alt+space and for the selected attachment alt+space. I've tested this now for three days and it works nice in most cases and I had no crash (so its very stable now). You need 10.6.

Comment 29

[Nomis101]

Attachment: Patch to include the QL-enabler extension into TB 3.3

Because the old builds from Comment 28 are obsolete, I've made new ones (in 64bit-only). I've attached the patch I've used to make this builds. I've asked Daniel and he saied he is OK with this. The original code is from https://bitbucket.org/libras2909/ql-enabler/src, but I've changed a few things.

• I've removed some whitespace
• I've aligned some lines properly
• I've added licence headers to the new files, with Daniel as the Contributor
• I've made the extension localizable
• Because I sometimes had permission problems while quicklooking a file, in QlXpcom.mm I've changed nsIFile::DIRECTORY_TYPE to 0777 and nsIFile::NORMAL_FILE_TYPE to 0666.
• Because it doesn't do anything for me, I've removed the menu file QL_menu_overlay.xul
• I've changed
> qlAllAttachments.setAttribute('hidden', !selectNone);
into
> qlAllAttachments.setAttribute('hidden', selectNone);
in msgHdrViewOverlay.js

Trybuilds (3.3a3pre, 20110216) are at:
http://tinyurl.com/5t27ya2 (de)
http://tinyurl.com/6yrhl5b (en-US)

Known issues with the patch: Will also build with Win and Linux, so needs some ifdefs. Because the accesskey to quicklook is not the same normally used in OS X (space), this maybe can confuse the user (but space is already used in TB).

Comment 30

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 512870 [details] [diff] [review]
Patch to include the QL-enabler extension into TB 3.3

Forwarding to bwinton for review (he was Daniel's mentor during GSoC), and asking for feedback from Daniel himself. Also running it by Bryan for ui-review.

Comment 31

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 512870 [details] [diff] [review]
Patch to include the QL-enabler extension into TB 3.3

I'm trying out the try server build and it looks pretty good so far.  I guess I haven't been much of a keyboard user in the attachments area so it was a little hard to get started.  Anyway, here are my comments.

* I had an odd issue where the (PDF) attachment wasn't downloaded at first.  Everything worked pretty well, the quick look showed the icon only and then transitioned into the full PDF quick look.  However I lost the keyboard shortcuts, like I couldn't use escape to close it anymore.

* The title for the quick look window is showing the file URI instead of the attachment file name.  We should get the file name showing instead.

In the context menu:
* I like the "Quick Look All" option, I don't think this needs the elipsis afterward.  Checking on my Mac and it doesn't use that for Quick Look.
* I would rename the "Quick Look These..." to be more consistent with the OS.  "Quick Look $FILENAME" for a single item and "Quick Look ## Items" for multiple items selected.


ui-r=me if those things get fixed.

Comment 32

[Charles Slivkoff]

For those of us stuck with a PPC-based Mac, is there any chance that this extension might be back-ported to work with 3.1?

Comment 33

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 512870 [details] [diff] [review]
Patch to include the QL-enabler extension into TB 3.3

>+++ b/mail/base/content/msgHdrViewOverlay.js
>@@ -2030,16 +2034,29 @@ function addAttachmentToPopup(popup, att
>+      // Append QL stuff
>+      var menuseparator = document.createElement('menuseparator');
>+      openpopup.appendChild(menuseparator);
>+
>+      menuitementry = document.createElement('menuitem');
>+      menuitementry.attachment = cloneAttachment(attachment);
>+      menuitementry.setAttribute('oncommand', 'qlpanel.previewAttachment(this.attachment)');
>+      menuitementry.setAttribute('label', getString("quicklookLabel"));
>+      menuitementry.setAttribute('accesskey', getString("quicklookAccesskey"));
>+      if (attachment.contentType == 'text/x-moz-deleted' || !canDetach || attachment.isExternalAttachment)
>+        menuitementry.setAttribute('disabled', true);
>+      menuitementry = openpopup.appendChild(menuitementry);

These should probably be split to be less than 80 characters.
I suggest something like:
      if (attachment.contentType == 'text/x-moz-deleted' ||
          !canDetach || attachment.isExternalAttachment)
        menuitementry.setAttribute('disabled', true);

>@@ -2118,16 +2136,20 @@ function HandleMultipleAttachments(attac
>+   else if (action == 'ql')
>+    {
>+	  qlpanel.previewMultipleAttachments(attachmentContentTypeArray, attachmentUrlArray, attachmentDisplayNameArray, attachmentMessageUriArray);
>+     }

That's some messed up indentation there, but to be consistent with the rest of the file, I think you should indent the qlpanel line an extra space, and remove the {}s.  And wrap the line to 80 characters.


>+++ b/mail/extensions/Makefile.in
>@@ -38,17 +38,17 @@
>-DIRS = mailviews
>+DIRS = mailviews qlxpcom

So, I wonder if we want to put this in the tree as an extension, or try to land it as a patch to the core instead.  I guess my vote is that landing it this way will improve things, and we can change it to an in-core patch later.

I would be interested in Standard8's opinions here, too.

>+++ b/mail/extensions/qlxpcom/Makefile.in
>+++ b/mail/extensions/qlxpcom/build/Makefile.in

Do we need both of these?

>+++ b/mail/extensions/qlxpcom/build/QlXpcomFactory.mm

This looks okay to me, but we should probably get someone else with more .mm experience to check it over.  Standard8?  Are you up for a second review of this patch?

>+++ b/mail/extensions/qlxpcom/content/QL_attachment_overlay.xul
>@@ -0,0 +1,59 @@
>+		<menuitem id="context-qlAllAttachments" label="&QuicklookAll.label;"
>+			  oncommand="HandleAllAttachments('ql');"/>
>+		<menuitem id="context-qlAttachments" label="&QuicklookThese.label;"
>+			  oncommand="HandleSelectedAttachments('ql');"/>
>+	</menupopup>
>+	<keyset>
>+	    <key id="key_qlAllAttachments" modifiers="control alt" key=" "
>+		 oncommand="HandleAllAttachments('ql');"/>
>+			<key id="key_qlAttachments" modifiers="alt" key=" "
>+		 oncommand="HandleSelectedAttachments('ql');"/>

The "oncommand"s should line up with the "id".

>+++ b/mail/extensions/qlxpcom/content/qlpanel.js
>@@ -0,0 +1,67 @@
>+var qlservice = Components.classes["@gsoc2010.com/qlxpcom;1"].createInstance();

I suspect we'll want to change the string "@gsoc2010.com/qlxpcom;1"" to something more descriptive, and less date-related.  ;)

>+dump(qlservice);

We should remove the dump statements, or turn them into log statements.

>+var qlpanel =
>+{
>+	toggleQL : function()
>+	{

Spaces, not tabs, please.  And the { should go on the same line as the function.

>+  	qlservice.openQL();
>+	},

So, if toggleQL always calls openQL, then it's not really toggling, is it?  (Or perhaps openQL toggles it, in which case, openQL is poorly named.  ;)

>+	previewMultipleAttachments : function(aAttachmentContentTypeArray, aAttachmentUrlArray, aAttachmentDisplayNameArray, aAttachmentMessageUriArray)

Please split this onto multiple lines.

>+	{
>+		qlservice.previewMany(aAttachmentContentTypeArray.length,
>+										aAttachmentContentTypeArray, aAttachmentUrlArray,
>+	                        aAttachmentDisplayNameArray, aAttachmentMessageUriArray);
>+	},

This indentation needs fixing, but that might be partly because my tab settings are different than yours.

>+  previewAttachment : function(aAttachment)
>+  {
>+		qlservice.previewOne(aAttachment.contentType,
>+                     aAttachment.url,
>+                     encodeURIComponent(aAttachment.displayName),
>+                     aAttachment.uri);
>+
>+		dump("------------- Preview-ing an attachment -------------\n");
>+  }

How is this different than calling the previous function and only passing in one-element arrays?  Can we merge the two functions, to get a simpler API?

>+++ b/mail/extensions/qlxpcom/content/qlxpcom.js
>@@ -0,0 +1,43 @@
>+/*
>+   Add any default pref values we want for smime
>+*/

Copy-paste error.  ;)
But, having said that, I think you can just delete the whole comment.

>+++ b/mail/extensions/qlxpcom/src/Makefile.in
>@@ -0,0 +1,69 @@
>+CMMSRCS          = \
>+                 QlXpcom.mm \
>+	         QlItem.mm \
>+		 QlData.mm \
>+		 $(NULL)

The indentation seems off here.

>+++ b/mail/extensions/qlxpcom/src/QlData.h
>@@ -0,0 +1,66 @@
>+- (void)previewOne:(const char*) aURL;
>+- (void)previewMany:(const char**) aURLArray length:(unsigned int) numItem;

Why not use NSArrays and NSStrings here?

>++ (id)sharedQlData;

So, sharedQlData isn't actually an id type, it's a QlData, correct?
I think you can use something like http://stackoverflow.com/questions/145154/what-does-your-objective-c-singleton-look-like/145164#145164 to make it more type-safe, and thread-safe.

>+- (void)previewOne:(const char*) aURL;
>+- (void)previewMany:(const char**) aURLArray length:(unsigned int) numItem;
>+
>+- (void)addToDict:(const char*) aURL withDiskURL:(const char*) aDiskURL;
>+- (BOOL)URLInDict:(const char*) aURL;

I think this should be "addURL:withDiskURL:", and "hasURL:", since the fact that you're using a dictionary is an implementation detail, and using "addToDict:" implies that the argument following that should be the dict that you're adding to.

>+++ b/mail/extensions/qlxpcom/src/QlData.mm
>@@ -0,0 +1,241 @@
>+#import "QlData.h"
>+#import "QlItem.h"
>+
>+// QlItem will be used directlty as the items in preview panel

typo: "directly"

>+@implementation QlData
>+
>++ (id)sharedQlData
>+{
>+		sharedQlData = [[self alloc] init];
>+		return sharedQlData;

I think this will cause a memory leak, because you alloc and init it here, but you don't release it in the dealloc method.

>+- (id)init
>+{
>+    self = [super init];

Too much indentation here.

>+- (void)tooglePreviewPanel:(id)previewPanel

You don't actually use the previewPanel in this method…
Which leads me to the question: Can we have more than one preview panel open at any given time?

>+- (BOOL)panelOpened
>+{
>+	return [QLPreviewPanel sharedPreviewPanelExists] && [[QLPreviewPanel sharedPreviewPanel] isVisible];

Please split this onto two lines to stay under 80 characters.

>+- (void)addToDict:(const char*) aURL withDiskURL:(const char*) aDiskURL
>+{
>+	NSString *diskURL;
>+
>+	// not in dictionary yet
>+	if (aDiskURL != NULL) {
>+		NSString *URL = [[NSString alloc] initWithCString:aURL encoding:NSMacOSRomanStringEncoding];
>+		diskURL = [[NSString alloc] initWithCString:aDiskURL encoding:NSMacOSRomanStringEncoding];

I'm slightly shocked that we aren't using NSUTF8StringEncoding.  Is there a reason we're not?

(Also, we need a bunch of lines wrapped at 80 characters.

>+- (void)previewMany:(const char**) aURLArray length:(unsigned int) numItem

This method looks like it doesn't actually preview the attachments, it just adds them to the selected attachment.  Is that correct, and if so, can we rename the method to something that indicates that?

>+- (void)setSelectedAttachment:(NSArray *)array
>+{
>+  array = [array copy];
>+  [selectedAttachment release];
>+  selectedAttachment = array;
>+  [previewPanel reloadData];
>+
>+	if (![self panelOpened])
>+		[self tooglePreviewPanel:previewPanel];

Oh, and there's my answer.  So I think this method needs renaming, then, to something like "setAndDisplayAttachments"…

>+- (BOOL)acceptsPreviewPanelControl:(QLPreviewPanel *)panel
>+{
>+	// NSLog(@"Accept Control");

We can probably remove this comment.  :)

>+++ b/mail/extensions/qlxpcom/src/QlItem.h
>@@ -0,0 +1,54 @@
>+- (id)initWithOriginalURL:(NSURL *)downloadURL fileURL:(NSURL *)onDiskURL;

It's traditional to name the arguments "aOriginalURL" and "aFileUrl" in this case.

>+++ b/mail/extensions/qlxpcom/src/QlXpcom.h
>@@ -0,0 +1,72 @@
>+#ifndef _QlXpcom_H_
>+#define _QlXpcom_H_

If you only reference this from .mm files, you can use:
#import "QlXpcom.h"
and it will do the dumb #ifndef trick for you.
(But it may be safer to leave it this way…)

>+++ b/mail/extensions/qlxpcom/src/QlXpcom.mm
>@@ -0,0 +1,188 @@
>+	if (dirExists) {
>+		dir->Remove(PR_TRUE); // remove our previous temp dir
>+		dir->Create(nsIFile::DIRECTORY_TYPE, 0777); // re-create it
>+	}
>+	else
>+		dir->Create(nsIFile::DIRECTORY_TYPE, 0777);

I'm always worried about directories that are world-writeable.  Could we change the permissions to 0700 instead?

>+	NSLog(@"-------------- QlXpcom died ------------");

This looks like we might want to log it to the error console instead.

>+		messenger->SaveAttachmentToFile(dir, aURL, aMessageUri, aContentType, nsnull);

Does that complete synchronously?  And if not, where do we wait for it to complete before we try to display the preview panel?

>+// void previewMany(in unsigned long count, [array, size_is(count)] in string contentTypeArray, [array, size_is(count)] in string urlArray, [array, size_is(count)] in string displayNameArray, [array, size_is(count)] in string messageUriArray);
>+NS_IMETHODIMP QlXpcom::PreviewMany(PRUint32 count, const char **contentTypeArray, const char **urlArray, const char **displayNameArray, const char **messageUriArray)
[snip…]
>+		else
>+			diskURL = NULL; // already in dictionary
>+
>+		[data addToDict:URL withDiskURL:diskURL];

If it's already in the dictionary, won't this call remove it from the dictionary?

Finally, testing it, I don't see the attachment menu items, but I do see at the console:
JavaScript error: chrome://messenger/content/msgHdrViewOverlay.js, line 1755: qlAttachments is null

Also, there are no tests.  So, given all of that, I'm going to have to give it an r-, but I do think that we should get Standard8's thoughts before you submit a new patch, just so that we can fix everything at the same time.

Thanks,
Blake.

Comment 34

[Nomis101]

(In reply to comment #33)
> Also, there are no tests.  So, given all of that, I'm going to have to give it
> an r-, but I do think that we should get Standard8's thoughts before you submit
> a new patch, just so that we can fix everything at the same time.

I think we either way need a third patch and can't fix everything at the first time. I can easily fix the most things you suggested, but for some things you and clarkbw suggested you need deeper coding knowledges. And I have no idea how to write tests. So I can wait what Standard8 says, than fix everything I can fix. And the rest must be fixed from a person with better coding knowledge.

Comment 35

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #34)
> > Also, there are no tests.
> I think we either way need a third patch and can't fix everything at the first
> time. I can easily fix the most things you suggested, but for some things you
> and clarkbw suggested you need deeper coding knowledges. And I have no idea how
> to write tests. So I can wait what Standard8 says, than fix everything I can
> fix. And the rest must be fixed from a person with better coding knowledge.

That sounds good to me.  Fix what you can, and leave a list of stuff still to do.

For the tests, I was thinking that we could start with manual testing, so all you'ld have to do is write down the steps you follow to test the extension, and the results you expect to see.  After that, someone can take those tests and automate them as a separate patch, or maybe even a separate bug.

Thanks,
Blake.

Comment 36

[Nomis101]

Attachment: Some testfiles for the QL feature

(In reply to comment #35)
> For the tests, I was thinking that we could start with manual testing, so all
> you'ld have to do is write down the steps you follow to test the extension, and
> the results you expect to see.  After that, someone can take those tests and
> automate them as a separate patch, or maybe even a separate bug.
> 
> Thanks,
> Blake.


This are some testfiles for the Quicklook feature. To use this for testing, do the following:

1. Download a QL-enabled TB build and the testfiles. Unzip the testfiles
2. Compose a new email and attach all of this testfiles
3. Safe this message as draft
4. Go into your drafts folder and drag&drop your email from the drafts folder to your inbox. Now you have an email with all files as attachments.
5. Open this email and quicklook the files. You can do this with a rightclick on it or with the shortcut ctrl+alt+space / alt+space

You have the following testfiles:
Testfile_CMYK.jpg --> This is a JPEG picture with a lavender color in CMYK color space. You should see a different color in TB compared to your QL preview.
Testfile_DOC.doc --> This is a MS Word testfile. If you have an Office.qlgenerator in /System/Library/QuickLook, you should see a purple text "This is a testfile for the Thunderbird Quicklook feature.". If you don't have this plugin, you should see the MS Word file icon.
Testfile_GIF.gif --> This is a picture testfile in GIF format, you should see a picture of an apple.
Testfile_IN.in --> This is an unsupported file format, so you should see a blank file icon.
Testfile_JPEG.jpg --> This is a JPEG picture file in the RGB colorspace, you should see a picture of an apple.
Testfile_MP3.mp3 --> This is an audio testfile in the MP3 format. You should hear the text "This is a testfile for the Thunderbird Quicklook feature."
Testfile_MP4.mp4 --> This is a video test file in MP4 v2 encoding. You should see a few seconds of movie.
Testfile_PDF.pdf --> This is a PDF test file. You should see a purple text "This is a testfile for the Thunderbird Quicklook feature.". 
Testfile_PNG.png --> This is a picture test file in the PNG format. You should see an apple.
Testfile_ZIP.zip --> This is an archive test file in the ZIP format. You should see the ZIP file icon. If you have an additional QL plugin for ZIP files installed, than you should see something different (depends on the plugin).

Comment 37

[Nomis101]

Attachment: Patch to include the QL-enabler extension into TB 3.3 (v.2)

What I fixed in the new patch:

* QL extension now only builds on mac
* Removed the elipse from "Quick Look All"
* Split some long lines into shorter lines
* Fixed some indentations and lined up some lines
* Renamed "@gsoc2010.com/qlxpcom;1" into "@mozilla.org/qlxpcom;1"
* Removed the dump statement "dump(qlservice);"
* Removed my C&P error in qlxpcom/content/qlxpcom.js
* Fixed the 'directlty' typo
* Now we are using NSUTF8StringEncoding
* Removed the "// NSLog(@"Accept Control");" comment
* Now the directories are not world-writeable anymore


What are not fixed in this patch, because I don't no how or I broken the code while trying to fix it:

* Make it possible to close the QL panel with escape
* The title for the quick look window is showing the file URI instead of the attachment file name.  We should get the file name showing instead.
* Rename "Quick Look These..." into "Quick Look $FILENAME" for a single item and "Quick Look ## Items" for multiple items selected.
* Land it as a core patch


>+++ b/mail/extensions/qlxpcom/Makefile.in
>+++ b/mail/extensions/qlxpcom/build/Makefile.in
Do we need both of these?


>+  	qlservice.openQL();
>+	},
So, if toggleQL always calls openQL, then it's not really toggling, is it?  (Or
perhaps openQL toggles it, in which case, openQL is poorly named.  ;)


>+  previewAttachment : function(aAttachment)
>+  {
>+		qlservice.previewOne(aAttachment.contentType,
>+                     aAttachment.url,
>+                     encodeURIComponent(aAttachment.displayName),
>+                     aAttachment.uri);
>+
>+		dump("------------- Preview-ing an attachment -------------\n");
>+  }
How is this different than calling the previous function and only passing in
one-element arrays?  Can we merge the two functions, to get a simpler API?


>+++ b/mail/extensions/qlxpcom/src/QlData.h
>@@ -0,0 +1,66 @@
>+- (void)previewOne:(const char*) aURL;
>+- (void)previewMany:(const char**) aURLArray length:(unsigned int) numItem;
Why not use NSArrays and NSStrings here?


>++ (id)sharedQlData;
So, sharedQlData isn't actually an id type, it's a QlData, correct?
I think you can use something like
http://stackoverflow.com/questions/145154/what-does-your-objective-c-singleton-look-like/145164#145164
to make it more type-safe, and thread-safe.


>+- (void)previewOne:(const char*) aURL;
>+- (void)previewMany:(const char**) aURLArray length:(unsigned int) numItem;
>+
>+- (void)addToDict:(const char*) aURL withDiskURL:(const char*) aDiskURL;
>+- (BOOL)URLInDict:(const char*) aURL;
I think this should be "addURL:withDiskURL:", and "hasURL:", since the fact
that you're using a dictionary is an implementation detail, and using
"addToDict:" implies that the argument following that should be the dict that
you're adding to.


>+@implementation QlData
>+
>++ (id)sharedQlData
>+{
>+		sharedQlData = [[self alloc] init];
>+		return sharedQlData;
I think this will cause a memory leak, because you alloc and init it here, but
you don't release it in the dealloc method.


>+- (void)tooglePreviewPanel:(id)previewPanel
You don't actually use the previewPanel in this method…
Which leads me to the question: Can we have more than one preview panel open at
any given time?


>+++ b/mail/extensions/qlxpcom/src/QlItem.h
>@@ -0,0 +1,54 @@
>+- (id)initWithOriginalURL:(NSURL *)downloadURL fileURL:(NSURL *)onDiskURL;
It's traditional to name the arguments "aOriginalURL" and "aFileUrl" in this
case.


>+	NSLog(@"-------------- QlXpcom died ------------");
This looks like we might want to log it to the error console instead.


>+		messenger->SaveAttachmentToFile(dir, aURL, aMessageUri, aContentType, nsnull);
Does that complete synchronously?  And if not, where do we wait for it to
complete before we try to display the preview panel?


>+// void previewMany(in unsigned long count, [array, size_is(count)] in string contentTypeArray, [array, size_is(count)] in string urlArray, [array, size_is(count)] in string displayNameArray, [array, size_is(count)] in string messageUriArray);
>+NS_IMETHODIMP QlXpcom::PreviewMany(PRUint32 count, const char **contentTypeArray, const char **urlArray, const char **displayNameArray, const char **messageUriArray)
[snip…]
>+		else
>+			diskURL = NULL; // already in dictionary
>+
>+		[data addToDict:URL withDiskURL:diskURL];
If it's already in the dictionary, won't this call remove it from the
dictionary?

Comment 38

[Mark Banner (:standard8)]

Attachment: Patch v3

I've discovered that QLPreviewPanelDataSource and QLPreviewPanelDelegate are 10.6 only API functions, therefore I've not been able to compile this knew patch. I've not yet investigated alternatives, but we'll need to be considering some method of managing this.

Changes I've made in this version:
- Move from mail/extensions/qlxpcom to mail/components/quicklook
-- The extensions code only really reflects mailnews/. No need for quicklook to be in there. Also wanted to drop the xpcom part - we know its xpcom ;-)
-- It also enables it to compile properly with our current libxul set-up.
- Renamed IQlXpcom to IQlService (and changed the relevant files/functions).
- Removed the preference to turn on/off.
-- This didn't seem to be used, nor to have much use once it is running.
- Moved the separate factory build code into mail/components/build.
- Added the new makefile to mail/makefiles.sh
- Documented IQlService.idl and corrected types of some of the functions.
- Fixed up tabs and layout in files.
- Revised some of QlService.mm so that it works better (more naturally) with the string APIs.

I've also added quite a few XXX comments - some of these are against issues that I've seen in the patch and haven't fixed yet, the rest of them are Blake's comments that are still outstanding.

I'll chat to Blake tomorrow about the best way we can move this forward from here.

Comment 39

[Nomis101]

Thanks for making this patch better. While removing the preference to turn on/off, it seems you missed to removed the reference from jar.mn:

 raise RuntimeError('File "%s" not found in %s' % (src, ', '.join(src_base)))
RuntimeError: File "content/qlxpcom.js" not found in /Developer/temp/src/mail/components/quicklook, .
make[6]: *** [libs] Error 1


And I get:

/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:10: fatal error: 
      'nsQlService.h' file not found
#include "nsQlService.h"
         ^
1 error generated.
make[6]: *** [nsMailComps.o] Error 1

Comment 40

[Nomis101]

(In reply to comment #38)
> I'll chat to Blake tomorrow about the best way we can move this forward from
> here.
Are there in the meantime any decisions how to move this forward? :-)

Comment 41

[Nomis101]

Bug 656045 and Bug 659205 made changes which also affects this patch.

Comment 42

[P. Bihler]

Wouldn't it be possible as a "quick hack" to write an add-on that reacts to pressing spacebar on n attachment, extracts this temporarly and uses the qlmanage command to display the attachment?

see http://www.macworld.com/article/131923/2008/02/qlterminal.html

++ Pascal

Comment 43

[Daniel]

(In reply to comment #42)
> Wouldn't it be possible as a "quick hack" to write an add-on that reacts to
> pressing spacebar on n attachment, extracts this temporarly and uses the
> qlmanage command to display the attachment?
> 
> see http://www.macworld.com/article/131923/2008/02/qlterminal.html
> 
> ++ Pascal

I had already considered using "qlmanage" when I first started working on this bug. I don't think this is desirable because there is no other way than clicking the "x" button at the top-left corner to close the pop-up preview window (when using the -p option). Moreover, you won't have the options to go full-screen or import the preview to iPhoto (if it's an image).

Comment 44

[Nomis101]

I think I was able to unbitrot the patch. But I'm not able to test if this unbitrot was correct, because now I again get the error I mentioned in #39

/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:10: fatal error: 
      'nsQlService.h' file not found
#include "nsQlService.h"

If I change nsQlService.h to QlService.h in nsMailComps.cpp (which is the name of the file) and include the path into its makefile, than I don't get this error anymore, but than I get another one. 
Does anyone know what is needed to bring this patch to compile successfully? I now spend one week or so with this...

Comment 45

[Markus Stange [:mstange]]

Changing it to QlService.h is correct. What's the other error? Try replacing all instances of "qlxpcom" with "qlservice" in nsMailComps.cpp, but keeping the same capitalization. For example, I can see that QlService.h #defines QLSERVICE_CID, so it must be NS_DEFINE_NAMED_CID(QLSERVICE_CID); in nsMailComps.cpp.

Comment 46

[Nomis101]

(In reply to comment #45)
> Changing it to QlService.h is correct. What's the other error? Try replacing
> all instances of "qlxpcom" with "qlservice" in nsMailComps.cpp, but keeping
> the same capitalization. For example, I can see that QlService.h #defines
> QLSERVICE_CID, so it must be NS_DEFINE_NAMED_CID(QLSERVICE_CID); in
> nsMailComps.cpp.
If did that now, but I still get this other error. This error is a bit long:

In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:189:1: error: 
      expected unqualified-id
@class NSString, Protocol;
^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:191:19: error: 
      unknown type name 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT NSString *NSStringFromSelector(SEL aSelector);
                  ^~~~~~~~
                  nsString
../../../mozilla/dist/include/nsTString.h:54:7: note: 'nsString' declared here
class nsTString_CharT : public nsTSubstring_CharT
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:42:45: note: expanded from:
#define nsTString_CharT                     nsString
                                            ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:192:44: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT SEL NSSelectorFromString(NSString *aSelectorName);
                                           ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:192:54: error: 
      use of undeclared identifier 'aSelectorName'
FOUNDATION_EXPORT SEL NSSelectorFromString(NSString *aSelectorName);
                                                     ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:194:19: error: 
      unknown type name 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT NSString *NSStringFromClass(Class aClass);
                  ^~~~~~~~
                  nsString
../../../mozilla/dist/include/nsTString.h:54:7: note: 'nsString' declared here
class nsTString_CharT : public nsTSubstring_CharT
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:42:45: note: expanded from:
#define nsTString_CharT                     nsString
                                            ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:195:43: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT Class NSClassFromString(NSString *aClassName);
                                          ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:195:53: error: 
      use of undeclared identifier 'aClassName'
FOUNDATION_EXPORT Class NSClassFromString(NSString *aClassName);
                                                    ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:197:19: error: 
      unknown type name 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                  ^~~~~~~~
                  nsString                                        
../../../mozilla/dist/include/nsTString.h:54:7: note: 'nsString' declared here
class nsTString_CharT : public nsTSubstring_CharT
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:42:45: note: expanded from:
#define nsTString_CharT                     nsString
                                            ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:197:50: error: 
      use of undeclared identifier 'Protocol'
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                                                 ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:197:60: error: 
      use of undeclared identifier 'proto'
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                                                           ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:198:19: error: 
      unknown type name 'Protocol'
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                  ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:198:50: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                                                 ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:198:60: error: 
      use of undeclared identifier 'namestr'
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                                                           ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:202:30: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                             ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:202:40: error: 
      use of undeclared identifier 'format'; did you mean 'normal'?
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                                       ^~~~~~
                                       normal
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:526:3: note: 
      'normal' declared here
  normal                        = 0,
  ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:202:52: error: 
      expected ';' after top level declarator
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                                                   ^
                                                   ;
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:203:31: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                              ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:203:41: error: 
      use of undeclared identifier 'format'; did you mean 'normal'?
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                                        ^~~~~~
                                        normal                
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:526:3: note: 
      'normal' declared here
  normal                        = 0,
  ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:203:62: error: 
      expected ';' after top level declarator
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                                                             ^
                                                             ;
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make[6]: *** [nsMailComps.o] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

Comment 47

[Markus Stange [:mstange]]

(In reply to comment #38)
> - Moved the separate factory build code into mail/components/build.

This problably needs to be undone. QlService.h needs to be compiled in Objective C mode, and that doesn't happen if its included in a .cpp file.

Comment 48

[Nomis101]

What I can say, if I move Marks patch from /mail/components/quicklook into /mail/extensions/quicklook, than (with minor changes) it builds without any error.

Comment 49

[Nomis101]

Attachment: Effort to make Marks patch build with current trunk

Currently I have two different patches in the pipeline. My current plan is, get a patch to build, than get it to work and than improve it (fix all xxx comments). 

This first patch is the above patch from Mark with the following changes:
- I removed the line with qlxpcom.js from jar.mn
- In nsMailComps.cpp I changed "nsQlService.h" to "QlService.h"
- I've added the quicklook folder to /components/build/Makefile.in
- I've tried to unbitrot msgHdrViewOverlay.js (I assume something there is not correct)
- I've changed all PRbool to bool
- I removed the "ifdef MOZ_ENABLE_LIBXUL" part from the makefile
- I readded some includefiles to QlService.h

This fixed a few errors I've seen. But now I've stuck with an error that I'm not able to fix:

In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:305:43: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT Class NSClassFromString(NSString *aClassName);
                                          ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:305:53: error: 
      use of undeclared identifier 'aClassName'
FOUNDATION_EXPORT Class NSClassFromString(NSString *aClassName);
                                                    ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:307:19: error: 
      unknown type name 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                  ^~~~~~~~
                  nsString                                        
../../../mozilla/dist/include/nsTString.h:54:7: note: 'nsString' declared here
class nsTString_CharT : public nsTSubstring_CharT
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:42:45: note: expanded from:
#define nsTString_CharT                     nsString
                                            ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:307:50: error: 
      use of undeclared identifier 'Protocol'
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                                                 ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:307:60: error: 
      use of undeclared identifier 'proto'
FOUNDATION_EXPORT NSString *NSStringFromProtocol(Protocol *proto) ...
                                                           ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:308:19: error: 
      unknown type name 'Protocol'
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                  ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:308:50: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                                                 ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:308:60: error: 
      use of undeclared identifier 'namestr'
FOUNDATION_EXPORT Protocol *NSProtocolFromString(NSString *namestr) ...
                                                           ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:312:30: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                             ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:312:40: error: 
      use of undeclared identifier 'format'; did you mean 'normal'?
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                                       ^~~~~~
                                       normal
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:526:3: note: 
      'normal' declared here
  normal                        = 0,
  ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:312:52: error: 
      expected ';' after top level declarator
FOUNDATION_EXPORT void NSLog(NSString *format, ...) NS_FORMAT_FUNCTION(1,2);
                                                   ^
                                                   ;
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:313:31: error: 
      use of undeclared identifier 'NSString'; did you mean 'nsString'?
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                              ^
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:313:41: error: 
      use of undeclared identifier 'format'; did you mean 'normal'?
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                                        ^~~~~~
                                        normal                
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:526:3: note: 
      'normal' declared here
  normal                        = 0,
  ^
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/nsMailComps.cpp:44:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlService.h:43:
In file included from /Users/polysom/Documents/Developer/temp/src/mail/components/build/../quicklook/QlData.h:39:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:
In file included from /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:8:
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:313:62: error: 
      expected ';' after top level declarator
FOUNDATION_EXPORT void NSLogv(NSString *format, va_list args) ...
                                                             ^
                                                             ;
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make[6]: *** [nsMailComps.o] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

Comment 50

[Nomis101]

Attachment: Mixed patch

This second patch is a mixture from Marks patch and the original patch. I've put it back into the extensions folder and in subfolders. This patch has the advantage that it builds fine with current trunk. :-) But it doesn't work. :-( I don't see the context menu and it doesn't quicklook. I think this is mainly because of mistakes in my unbitrotted msgHdrViewOverlay.js

I also see the following in TBs error console:

Error: Components.classes['@mozilla.org/qlservice;1'] is undefined
Source File: chrome://messenger-qlservice/content/qlpanel.js
Line: 39

Error: qlAttachment is not defined
Source File: chrome://messenger/content/msgHdrViewOverlay.js
Line: 1823

Error: this.attachment.qlpanel is undefined
Source File: chrome://messenger/content/messageWindow.xul
Line: 1

Comment 51

[Markus Stange [:mstange]]

(In reply to Nomis101 from comment #49)
> In file included from
> /Users/polysom/Documents/Developer/temp/src/mail/components/build/
> nsMailComps.cpp:44:
> In file included from
> /Users/polysom/Documents/Developer/temp/src/mail/components/build/../
> quicklook/QlService.h:43:
> In file included from
> /Users/polysom/Documents/Developer/temp/src/mail/components/build/../
> quicklook/QlData.h:39:
> In file included from
> /Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/Cocoa.framework/
> Headers/Cocoa.h:12:

Cocoa.h (and thus QlService.h) can't be included in a .cpp file, only in a .mm file.
That's why it doesn't work with nsMailComps.cpp but it does work with QlServiceFactory.mm (what you're doing in the other patch). This is the same thing that I mentioned in comment 47.

(In reply to Nomis101 from comment #50)
> Error: Components.classes['@mozilla.org/qlservice;1'] is undefined
> Source File: chrome://messenger-qlservice/content/qlpanel.js
> Line: 39

This is the error you absolutely need to get rid of before anything can work. It means that the QlService module doesn't exist. Maybe the factory didn't work for some reason. I don't see the problem at the moment, though.

Comment 52

[Nomis101]

(In reply to Markus Stange from comment #51) 
> Cocoa.h (and thus QlService.h) can't be included in a .cpp file, only in a
> .mm file.
> That's why it doesn't work with nsMailComps.cpp but it does work with
> QlServiceFactory.mm

Is there any solution for this? Changing nsMailComps.cpp to nsMailComps.mm gives me other errors.

Comment 53

[Markus Stange [:mstange]]

(In reply to Nomis101 from comment #52)
> (In reply to Markus Stange from comment #51) 
> > Cocoa.h (and thus QlService.h) can't be included in a .cpp file, only in a
> > .mm file.
> > That's why it doesn't work with nsMailComps.cpp but it does work with
> > QlServiceFactory.mm
> 
> Is there any solution for this?

Yes: Reverting the merge of QlServiceFactory.mm and nsMailComps.cpp. And as far as I can tell, that's what your "Mixed patch" does.

Comment 54

[Mark Banner (:standard8)]

Attachment: Patch that semi-compiles

This semi compiles if you s/QuartzCore/Quartz/ in the mozilla-central configure.in. That's a potentially big change for linking XUL against, so we might want to move that to a separate library and just link that library against Quartz.

Comment 55

[Nomis101]

What is macMailComps.mm? Couldn't find that file in the patch. And what does it mean if you say "s/QuartzCore/Quartz/"?

Comment 56

[Blake Winton (:bwinton) (:☕️)]

I'm not sure about the first part, but the second bit comes from vim, it means "Replace QuartzCore with Quartz".

Comment 57

[Nomis101]

Attachment: The macMailComps.mm I've used

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #56)
> I'm not sure about the first part, but the second bit comes from vim, it
> means "Replace QuartzCore with Quartz".

Thanks, I've did this and I've created a macMailComps.mm. Now it builds until it stops with:

ld: warning: could not create compact unwind for _ffi_call_unix64: does not use RBP or RSP based frame
Undefined symbols for architecture x86_64:
  "_macMailComps_NSModule", referenced from:
      _kPStaticModules in nsStaticXULComponents.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[5]: *** [XUL] Error 1
make[4]: *** [libs_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

Do I miss something in the macMailComps.mm?

Comment 58

[Ludovic Hirlimann [:Usul]]

(In reply to Nomis101 from comment #57)
> Thanks, I've did this and I've created a macMailComps.mm. Now it builds
> until it stops with:
> 
> ld: warning: could not create compact unwind for _ffi_call_unix64: does not
> use RBP or RSP based frame
> Undefined symbols for architecture x86_64:
>   "_macMailComps_NSModule", referenced from:
>       _kPStaticModules in nsStaticXULComponents.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> make[5]: *** [XUL] Error 1
> make[4]: *** [libs_tier_platform] Error 2
> make[3]: *** [tier_platform] Error 2
> make[2]: *** [default] Error 2
> make[1]: *** [default] Error 2
> make: *** [build] Error 2


Rafael anything obviuous here ?

Comment 59

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

> ld: warning: could not create compact unwind for _ffi_call_unix64: does not
> use RBP or RSP based frame

I have seen this before, so it is probably unrelated to the failure.

> Undefined symbols for architecture x86_64:
>   "_macMailComps_NSModule", referenced from:
>       _kPStaticModules in nsStaticXULComponents.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)

I have never built thunderbird myself, so I have no idea if some object should be defining _macMailComps_NSModule or if _kPStaticModules should not be referencing it.

Comment 60

[Nomis101]

Attachment: Marks semi-compile patch with the macMailComps.mm I've used

I found out why I didn't see the entrys in the contextual menu. After adding this.parentNode.attachments to the oncommand lines, I now see it. This patch fixes this. It also adds the macMailComps.mm I've used. But it still stops with the error from Comment 57. Currently I don't know why. (But it has nothing to do with Clang!)

Comment 61

[Andrea Monni]

This is one of the most requested features in GetSatisfaction with 80 votes and it's begin discussed here:

http://getsatisfaction.com/mozilla_messaging/topics/attachment_preview_add_on_for_thunderbird

Comment 62

[Andrea Monni]

This is one of the most requested features in GetSatisfaction with 80 votes and it's being discussed here:

http://getsatisfaction.com/mozilla_messaging/topics/attachment_preview_add_on_for_thunderbird

Comment 63

[Nomis101]

If somebody has an idea how to fix the compile issue and how to bring this patch forward, I'm disposed to test this out. I currently have a bit of time in my holidays. :-)

Comment 64

[Nomis101]

Attachment: Marks semi-compile patch with the macMailComps.mm I've used (v.1.1)

This is the patch I currently have. It fixes some minor issues I've found, it changes (PR_TRUE) to (true) and it is unbitrotted. It still doesn't build and you still need the Quartz changes from #54. I hope I get it to build this week.

Comment 65

[Mark Banner (:standard8)]

I'm going to take a look at this later today - I've revised my ideas about how we should link this so I'll see if I can make them work.

Comment 66

[Mark Banner (:standard8)]

Attachment: Updated patch

This updates the patch and gets us building again. As it is, I've actually gone back to building the service as a separate component. I think there's still plenty of tidy up to be done - especially the interaction between how we overlay items on the context menus for attachments.

I'll be doing some more on trying to get this a bit more functional and Mac specific tomorrow.

Comment 67

[kiwiplant]

Does this patch end up in nightly builds 11.0a2 ?  Wondering if there is a way to try it out without building locally from source.

Comment 68

[Nomis101]

(In reply to kiwiplant from comment #67)
> Does this patch end up in nightly builds 11.0a2 ?  Wondering if there is a
> way to try it out without building locally from source.
No, not yet, this patch is far from being ready for a nighly. You need to build TB with this patch for yourself. But I can send you a link of an unofficial trunk build I will do now with this patch, if you want.

Comment 69

[Mark Banner (:standard8)]

We can get special builds created, but this patch isn't at the stage where it would be beneficial to generate them - the UI only really works in one instance and how it is all hooked together isn't right either. I suggest just waiting a bit longer until its in slightly better shape.

Comment 70

[Nomis101]

Does this patch build for you? I get:
error: no such file or directory: '/Volumes/Developer/comm-central/obj-x86_64-apple-darwin11.3.0/mozilla/dist/bin/XUL'
make[6]: *** [libquicklook.dylib] Error 1
Do I need to modify something first?

Comment 71

[Mark Banner (:standard8)]

Attachment: Updated patch v2

This patch changes the UI so that we're now calling everything "Preview" in the front end and behind the scenes. It also checks for the quicklook service before displaying menu options - this will allow different platforms to hook in if required, and the possibility for extensions to provide implementations.

There's still a bit of work to do on the UI, but it is in a better shape now. The backend still feels like it needs a lot of work. I'll hopefully address a couple of things tomorrow, and then we can see where we go from there.

Comment 72

[Mark Banner (:standard8)]

(In reply to Nomis101 from comment #70)
> Does this patch build for you? I get:
> error: no such file or directory:
> '/Volumes/Developer/comm-central/obj-x86_64-apple-darwin11.3.0/mozilla/dist/
> bin/XUL'
> make[6]: *** [libquicklook.dylib] Error 1
> Do I need to modify something first?

That's probably an issue for clobber builds. I think I know how to fix it - I'll take a look tomorrow.

Comment 73

[Mark Banner (:standard8)]

Attachment: Updated patch v3

This patch does some pretty big restructuring:

- Renamed the code from quicklook to filepreview - its possible other platforms (or extensions) might want to implement something similar in future, so having a generic name makes that conceptually easier.
- Gets the Mac service to only be compiled on Mac and not on 10.5 as the interfaces aren't supported there.
- Adds "Preview" to all of the attachment menus if the service is present (i.e. on non-Mac platforms, it'll be hidden).

Obvious bugs/issues:

- Need to double-check all the preview items are hidden if the service isn't available.
- Try and work out why the preview window shows summary of file details sometimes and not others - e.g. when you view multiple attachments, you get a summary of the file details for the first attachment, but switching via the preview window looses those details.
- Implement a proper title on the preview window.
- Sort out if the shortcut keys work/are the ones we want.

Other things to fix up:

- I'm still not quite happy with the IFilePreviewService interfaces.
- Back-end code still needs lots of tidy up (style, variables used, check for memory issues etc).

Comment 74

[Nomis101]

Cool! :-) Your new patch builds just fine for a clobber build. I will play a bit with this build to see if I see something unexpected.

(In reply to Mark Banner (:standard8) (afk until 3rd Jan) from comment #73)
> - Try and work out why the preview window shows summary of file details
> sometimes and not others - e.g. when you view multiple attachments, you get
> a summary of the file details for the first attachment, but switching via
> the preview window looses those details.
Yes, I've also seen this with the original patch inside TB 3 to TB 6. I sometimes had the problem that, instead of quicklooking the file, I got only the file icon (like you see it in the finder). With your new patch I also see this, but I also see a new issue, that instead of quicklooking I see a blank file icon with the description on the right (like you get it for unsupported file types in the finder).

(In reply to Mark Banner (:standard8) (afk until 3rd Jan) from comment #73)
> - Sort out if the shortcut keys work/are the ones we want.
The current shortcut key is indeed hard to remember and hard to use. Therefore I only use the right-click menu.

Comment 75

[Nomis101]

Just a nit, in package-manifest.in it now also should be
>+@BINPATH@/components/libfilepreview.dylib
>+@BINPATH@/components/filepreview.xpt

Comment 76

[Nomis101]

Were does the quicklooked files go to? I see the download window popping up shortly while quicklooking, but were does it put the file? As expected it creates a folder /Users/Library/Caches/TemporaryItems/Thunderbird-Preview, but this folder is empty. There are no files inside while quicklooking.
I also sometimes get a crash while quicklooking (mostly for PDF files), I can debbug this with gdb if it helps.

Comment 77

[Mark Banner (:standard8)]

(In reply to Nomis101 from comment #76)
> I see the download window popping up
> shortly while quicklooking, but were does it put the file?

Oh, I don't see that. I suspect this is a side-effect of using SaveAttachmentToFile - we might need to use something different there.

> As expected it
> creates a folder /Users/Library/Caches/TemporaryItems/Thunderbird-Preview,
> but this folder is empty. There are no files inside while quicklooking.

I actually used "watch ls <dir>" and could see files appearing & disappearing. 

> I also sometimes get a crash while quicklooking (mostly for PDF files), I
> can debug this with gdb if it helps.

My suspicion is the file isn't present on disk when you're trying to quicklook it - do you use the imap offline sync option in Thunderbird (or local folders/pop)?

If SaveAttachmentToFile is the right thing, then one of the XXX's we need to fix is checking when that download finishes (if applicable), although I'm not quite sure what the UX should be there yet.

Comment 78

[Nomis101]

(In reply to Mark Banner (:standard8) (afk until 3rd Jan) from comment #77)
> My suspicion is the file isn't present on disk when you're trying to
> quicklook it - do you use the imap offline sync option in Thunderbird (or
> local folders/pop)?
I use POP, I haven't checked the QL support with IMAP yet.

I've also noticed the following. I've quicklooked a JPEG file and hit the button (in Lion) to open this file with Apples Preview application, what it does. But if I than try to save the file within Preview, I get a message that says, that the location of the file could not be determined. If I quicklook a PDF file and hit the button to open this file in Adobe Reader, nothing happens.

Comment 79

[Nomis101]

You were right, this patch is much better with IMAP. Today I've tried with IMAP and with IMAP I don't see the download window popping up shortly. And I also see no blank file icons while quicklooking with IMAP.

Comment 80

[Nomis101]

Attachment: Updated patch v3 to current trunk

The patch was slightly bitrotted, so I've unbitrotted it. During that I've also updated the files to MPL2 and I've changed DIRECTORY_TYPE to 755 (I had troubles with 700 and 755 is more often used in the mozilla code).

Comment 81

[Nomis101]

Attachment: Updated patch v3 to current trunk 2012-08-26

This is just a maintenance update, so that it will still build and work with current trunk (switch from nsnull to nullptr, from PRUint32 to uint32_t and so on and some unbitrotting).

Comment 82

[Ludovic Hirlimann [:Usul]]

Nomis101 what's left to do for this bug ?

Comment 83

[Nomis101]

(In reply to Ludovic Hirlimann [:Usul] from comment #82)
> Nomis101 what's left to do for this bug ?

First, we need some tests for this. Than we need to make QL work in all cases. Currently it works mostly in IMAP case and not so well in POP case. And the code needs to be made nice, so that it will pass a review and ui-r. What is needed for that is mostly written in #73:

> - Need to double-check all the preview items are hidden if the service isn't
> available.
> - Try and work out why the preview window shows summary of file details
> sometimes and not others - e.g. when you view multiple attachments, you get
> a summary of the file details for the first attachment, but switching via
> the preview window looses those details.
> - Implement a proper title on the preview window.
> - Sort out if the shortcut keys work/are the ones we want.
> 
> Other things to fix up:
> 
> - I'm still not quite happy with the IFilePreviewService interfaces.
> - Back-end code still needs lots of tidy up (style, variables used, check
> for memory issues etc).


For somebody with good coding skills this should be easy to do. I can't do all this, but I volunteer as a tester for every new patch.

Comment 84

[Nomis101]

Attachment: Updated patch v3 to current trunk 2012-09-29

nsCAutoString->nsAutoCString

Comment 85

[Nomis101]

Attachment: First attempt to move patch v3 to moz.build

I've tried to move the patch to moz.build. It builds without any error, but TB has no filepreview function at the end (don't know why).

Comment 86

[Nomis101]

Attachment: Updated patch v3 to current trunk 2013-11-10

This now builds with current trunk. I fixed the moz.build and nullptr stuff. I really hope somebody will take this bug and fix it...

Comment 87

[Nomis101]

Attachment: Updated patch v3 to current beta (TB35)

I had some time, so I've unbitroted patch v3 again. It now applies to current beta (TB 35). I fixed all moz.build related errors. But in the meantime the patch does not build anymore. It stopps with the error:

Developer/comm-beta/obj-x86_64-apple-darwin14.0.0/dist/private/nss  alg1485.c
/Volumes/Developer/comm-beta/mail/filepreview/MacFilePreviewService.mm:15:62: error: 
      parameter type 'IFilePreviewService' is an abstract class
NS_IMPL_ISUPPORTS1(MacFilePreviewService, IFilePreviewService)
                                                             ^
../../dist/include/nsISupportsBase.h:59:14: note: unimplemented pure virtual
      method 'QueryInterface' in 'IFilePreviewService'
  NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) = 0;
             ^
../../dist/include/nsISupportsBase.h:67:40: note: unimplemented pure virtual
      method 'AddRef' in 'IFilePreviewService'
  NS_IMETHOD_(MozExternalRefCountType) AddRef(void) = 0;
                                       ^
../../dist/include/nsISupportsBase.h:76:40: note: unimplemented pure virtual
      method 'Release' in 'IFilePreviewService'
  NS_IMETHOD_(MozExternalRefCountType) Release(void) = 0;
                                       ^
../../dist/include/IFilePreviewService.h:31:14: note: unimplemented pure virtual
      method 'GetPanelOpened' in 'IFilePreviewService'
  NS_IMETHOD GetPanelOpened(bool *aPanelOpened) = 0;
             ^
../../dist/include/IFilePreviewService.h:34:14: note: unimplemented pure virtual
      method 'TogglePanel' in 'IFilePreviewService'
  NS_IMETHOD TogglePanel(void) = 0;
             ^
../../dist/include/IFilePreviewService.h:37:14: note: unimplemented pure virtual
      method 'PreviewOne' in 'IFilePreviewService'
  NS_IMETHOD PreviewOne(const nsACString & aContentType, const nsACStrin...
             ^
../../dist/include/IFilePreviewService.h:40:14: note: unimplemented pure virtual
      method 'PreviewMany' in 'IFilePreviewService'
  NS_IMETHOD PreviewMany(uint32_t count, const char * *contentTypeArray...
             ^
/Volumes/Developer/comm-beta/mail/filepreview/MacFilePreviewService.mm:15:1: error: 
      C++ requires a type specifier for all declarations
NS_IMPL_ISUPPORTS1(MacFilePreviewService, IFilePreviewService)
^~~~~~~~~~~~~~~~~~
/Volumes/Developer/comm-beta/mail/filepreview/MacFilePreviewService.mm:17:24: error: 
      expected ';' after top level declarator
MacFilePreviewService::MacFilePreviewService()
                       ^
                       ;
/Volumes/Developer/comm-beta/mail/filepreview/MacFilePreviewService.mm:17:46: error: 
      expected unqualified-id
MacFilePreviewService::MacFilePreviewService()
                                             ^
4 errors generated.
make[5]: *** [MacFilePreviewService.o] Error 1
make[4]: *** [mail/filepreview/target] Error 2
make[4]: *** Waiting for unfinished jobs....


I have no idea what this means, so I don't think I can fix this and will therefore not unbitrot this patch again. :-(

Comment 88

[Wayne Mery (:wsmwk)]

(In reply to Nomis101 from comment #87)
> Created attachment 8539449 [details] [diff] [review]
> Updated patch v3 to current beta (TB35)
> 
> I had some time, so I've unbitroted patch v3 again. It now applies to
> current beta (TB 35). I fixed all moz.build related errors. But in the
> meantime the patch does not build anymore. It stopps with the error:
> 
> Developer/comm-beta/obj-x86_64-apple-darwin14.0.0/dist/private/nss  alg1485.c
> /Volumes/Developer/comm-beta/mail/filepreview/MacFilePreviewService.mm:15:62:
> error: 

Daniel, ,thoughts?

Comment 89

[jidiculous]

is this still ongoing?

Comment 90

[Nomis101]

(In reply to jidiculous from comment #89)
> is this still ongoing?

Currently nobody is working on it. If you are a software engineer, feel free to work on it. :-)

Comment 91

[jidiculous]

(In reply to Nomis101 from comment #90)
> (In reply to jidiculous from comment #89)
> > is this still ongoing?
> 
> Currently nobody is working on it. If you are a software engineer, feel free
> to work on it. :-)

Unfortunately I am not, but thank you for your reply :)