From: Thierry G. <mel...@ya...> - 2008-01-24 10:07:21
Attachments:
squirrelmail-20080115-SVN_devel-XSMTP.patch
|
Greetings, As I am developing a plugin which relies on X-SMTP headers (X-P772 standard, to be more precise), I needed for a better support of these headers in Squirrelmail, which for now mostly ignores them. The result of this work is the attached tiny patch which: - adds X-P772-Primary-Precedence support (this is the X-P772 equivalent to X-Priority, Importance and Priority); - makes Rfc822Header.class store any X-SMTP header it parses in its more_headers array, instead of discarding them. Regards, Thierry Godefroy. ____________________________________________________________________________________ Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping |
From: Thijs K. <ki...@sq...> - 2008-01-24 10:46:39
|
Hi Thierry, On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > As I am developing a plugin which relies on X-SMTP headers (X-P772 > standard, to be more precise), I needed for a better support of these > headers in Squirrelmail, which for now mostly ignores them. > > The result of this work is the attached tiny patch which: > - adds X-P772-Primary-Precedence support (this is the X-P772 equivalent to > X-Priority, Importance and Priority); > - makes Rfc822Header.class store any X-SMTP header it parses in its > more_headers array, instead of discarding them. Thank you for your patch. I'm a bit reserved about this one, because it add= s=20 support for a standard which is specific to the French military. It is only= =20 published in French and not through any official standards body. I think th= is=20 is really the kind of thing that should be implemented in plugins as much a= s=20 possible. I realise that parsing headers is currently suboptimal from a=20 plugin, but I'd rather see that improved in general than to add such specif= ic=20 headers to our code base... Thijs |
From: Thierry G. <mel...@ya...> - 2008-01-24 13:20:13
|
--- Thijs Kinkhorst <ki...@sq...> wrote: > Hi Thierry, > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > standard, to be more precise), I needed for a better support of these > > headers in Squirrelmail, which for now mostly ignores them. > > > > The result of this work is the attached tiny patch which: > > - adds X-P772-Primary-Precedence support (this is the X-P772 equivalent to > > X-Priority, Importance and Priority); > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > more_headers array, instead of discarding them. > > Thank you for your patch. I'm a bit reserved about this one, because it adds > support for a standard which is specific to the French military. It is only > published in French and not through any official standards body. The fact it was specified by a French entity (the DGA, which makes and sells military products all over the world), does not mean it is not an international standard... And even if the DGA never sells a product with this standard implemented to a foreign country, X-P772 could well nevertheless become a NATO standard (STANAG) as need for interoperability with other Armies will arise... > I think this is really the kind of thing that should be implemented in > plugins as much as possible. Not possible, or at least not without duplicating a lot of code where the 4 lines patch I gave you can do (and does) the trick. The priority is needed to display the proper flag in the mailbox message list. > I realise that parsing headers is currently suboptimal from a > plugin, but I'd rather see that improved in general than to add such specific > headers to our code base... An alternative would be to use an array of constants (which would include X-Priority as a default), defined in the default_config, for example, and treat all those X-Something-Priority fields in the same way... Yet, please, do not reject the whole patch: one part of it is independent of the X-P772 standard and allows to take into account any X-SMTP field: @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } Regards, Thierry Godefroy. ____________________________________________________________________________________ Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs |
From: Thijs K. <ki...@sq...> - 2008-01-24 15:03:52
|
On Thursday 24 January 2008 14:20, Thierry Godefroy wrote: > The fact it was specified by a French entity (the DGA, which makes and > sells military products all over the world), does not mean it is not an > international standard... And even if the DGA never sells a product with > this standard implemented to a foreign country, X-P772 could well > nevertheless become a NATO standard (STANAG) as need for interoperability > with other Armies will arise... What I meant was the following. If I Google for that standard, I find exact= ly=20 one document on the whole web that describes how to implement it, and it is= =20 written in the French language. I also do not find a single other reference= =20 to something publically implementing it. I do not say it's unimportant, but it is a protocol that is hence very limi= ted=20 in scope and as such, it's individual headers should not be added to=20 SquirrelMail core. I am all in favour of changing the core code to be more= =20 flexible however, so we can support this, and any other specialised=20 protocols. I think this is what Alexandros is willing to work on. Thijs |
From: Alexandros V. <av...@no...> - 2008-01-24 11:07:47
|
On Thu, 24 Jan 2008 11:45:44 +0100 Thijs Kinkhorst <ki...@sq...> wrote: > Hi Thierry, > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > standard, to be more precise), I needed for a better support of > > these headers in Squirrelmail, which for now mostly ignores them. > > > > The result of this work is the attached tiny patch which: > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > equivalent to X-Priority, Importance and Priority); > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > more_headers array, instead of discarding them. > > Thank you for your patch. I'm a bit reserved about this one, because > it adds support for a standard which is specific to the French > military. It is only published in French and not through any official > standards body. I think this is really the kind of thing that should > be implemented in plugins as much as possible. I realise that parsing > headers is currently suboptimal from a plugin, but I'd rather see > that improved in general than to add such specific headers to our > code base... Thijs, I had the exact same issue with an in-house patch of ours, targetted towards marking SPAM messages in folders. What it did was check for X-Spam-<foo> headers in messages inserted previously by sendmail, in a patch that is very similar to the one by Thierry Godefroy. Avelsieve's spam rule function inserted automatically a custom highlight rule that changed that messages' color, depending on that header. My point is that there is a need for things like my patch and Thierry's patch, to make this functionality accessible to plugins. Would anyone mind if I build the plugin architecture for this? I think it's something like three more hooks needed. Is it that terrible that we place such hooks in Rfc822Header.class.php? -- Alexandros Vellis National and Kapodistrian University of Athens Network Operations Centre |
From: Thijs K. <ki...@sq...> - 2008-01-24 11:10:42
|
On Thursday 24 January 2008 12:07, Alexandros Vellis wrote: > My point is that there is a need for things like my patch and Thierry's > patch, to make this functionality accessible to plugins. Yes, that's also my point :-) That's why I didn't want to add the specific= =20 extra headers. > Would anyone mind if I build the plugin architecture for this? I think > it's something like three more hooks needed. Is it that terrible that we > place such hooks in Rfc822Header.class.php? I think that's just fine and it would be great if you could work on this.=20 Maybe we can just have a hook that allows you to specify a list of extra=20 headers to grab? Thijs |
From: Alexandros V. <av...@no...> - 2008-01-24 11:48:47
|
On Thu, 24 Jan 2008 12:10:23 +0100 Thijs Kinkhorst <ki...@sq...> wrote: > > Would anyone mind if I build the plugin architecture for this? I > > think it's something like three more hooks needed. Is it that > > terrible that we place such hooks in Rfc822Header.class.php? > > I think that's just fine and it would be great if you could work on > this. Maybe we can just have a hook that allows you to specify a list > of extra headers to grab? OK, I'm taking this. I don't remember exactly since I wrote this a year ago, but I think there is something more to it than just defining the header names. I'll come back to you later. -- Alexandros Vellis National and Kapodistrian University of Athens Network Operations Centre |
From: Thierry G. <mel...@ya...> - 2008-01-24 13:37:56
|
On Thu, 24 Jan 2008 12:10:23 +0100 Thijs Kinkhorst <ki...@sq...> wrote: > > Would anyone mind if I build the plugin architecture for this? I > > think it's something like three more hooks needed. Is it that > > terrible that we place such hooks in Rfc822Header.class.php? > > I think that's just fine and it would be great if you could work on > this. Maybe we can just have a hook that allows you to specify a list > of extra headers to grab? The few necessary changes already exist in the patches I just submitted: In squirrelmail-20080115-SVN_devel-MelindaHooks.patch: diff -urN squirrelmail/functions/mailbox_display.php squirrelmail-patched/functions/mailbox_display.php --- squirrelmail/functions/mailbox_display.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 10:01:37.000000000 +0100 @@ -315,6 +315,9 @@ } } + // For X-SMTP headers we could need to fetch: + do_hook('fetch_more_headers', &$aHeaderFields); + /** * A uidset with sorted uid's is available. We can use the cache */ In squirrelmail-20080115-SVN_devel-XSMTP.patch: diff -urN squirrelmail/class/mime/Rfc822Header.class.php squirrelmail-melinda/class/mime/Rfc822Header.class.php --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 16:34:17.000000000 +0100 +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 16:09:56.000000000 +0100 @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } Regards, Thierry Godefroy. ____________________________________________________________________________________ Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs |
From: Paul L. <pa...@sq...> - 2008-01-29 22:22:07
|
On Jan 24, 2008 5:36 AM, Thierry Godefroy <mel...@ya...> wrote: > On Thu, 24 Jan 2008 12:10:23 +0100 > Thijs Kinkhorst <ki...@sq...> wrote: > > > > Would anyone mind if I build the plugin architecture for this? I > > > think it's something like three more hooks needed. Is it that > > > terrible that we place such hooks in Rfc822Header.class.php? > > > > I think that's just fine and it would be great if you could work on > > this. Maybe we can just have a hook that allows you to specify a list > > of extra headers to grab? > > The few necessary changes already exist in the patches I just submitted: > > In squirrelmail-20080115-SVN_devel-MelindaHooks.patch: > > > diff -urN squirrelmail/functions/mailbox_display.php > squirrelmail-patched/functions/mailbox_display.php > --- squirrelmail/functions/mailbox_display.php 2008-01-15 16:34:31.000000000 > +0100 > +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 > 10:01:37.000000000 +0100 > @@ -315,6 +315,9 @@ > } > } > > + // For X-SMTP headers we could need to fetch: > + do_hook('fetch_more_headers', &$aHeaderFields); > + > /** > * A uidset with sorted uid's is available. We can use the cache > */ Along with the other part of the original patch I agree with, I think this one is also correct and should be added. I would add them now, but I don't want to step on Alexandros' toes. (but I cannot imagine a more simple way to handle this...? Alexandros, are you doing something different/more complex?) |
From: Alexandros V. <av...@no...> - 2008-01-29 22:53:55
|
On Tue, 29 Jan 2008 14:22:02 -0800 "Paul Lesniewski" <pa...@sq...> wrote: (snip) > Along with the other part of the original patch I agree with, I think > this one is also correct and should be added. I would add them now, > but I don't want to step on Alexandros' toes. (but I cannot imagine a > more simple way to handle this...? Alexandros, are you doing > something different/more complex?) Hey Paul, I was looking at this whole thing just today. The spirit of the patch is correct, I made it a tad more general and it works well. I will post details on Friday. Indeed there was no end to touch Rfc822 class so far. However, this will only work for message list currently (right_main). I imagine that a plugin might want to do more stuff with extra headers in message display (read_body). That's where I got a bit stuck. Things are a bit different there, I think. Anyway, I'll be away for two days, so I'll try to get the first part out here for review on Friday. Alexandros |
From: Paul L. <pa...@sq...> - 2008-01-29 22:15:14
|
On Jan 24, 2008 3:07 AM, Alexandros Vellis <av...@no...> wrote: > > On Thu, 24 Jan 2008 11:45:44 +0100 > Thijs Kinkhorst <ki...@sq...> wrote: > > > Hi Thierry, > > > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > > standard, to be more precise), I needed for a better support of > > > these headers in Squirrelmail, which for now mostly ignores them. > > > > > > The result of this work is the attached tiny patch which: > > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > > equivalent to X-Priority, Importance and Priority); > > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > > more_headers array, instead of discarding them. > > > > Thank you for your patch. I'm a bit reserved about this one, because > > it adds support for a standard which is specific to the French > > military. It is only published in French and not through any official > > standards body. I think this is really the kind of thing that should I agree > > be implemented in plugins as much as possible. I realise that parsing > > headers is currently suboptimal from a plugin, but I'd rather see > > that improved in general than to add such specific headers to our > > code base... Right... > I had the exact same issue with an in-house patch of ours, targetted > towards marking SPAM messages in folders. > > What it did was check for X-Spam-<foo> headers in messages inserted > previously by sendmail, in a patch that is very similar to the one by > Thierry Godefroy. Avelsieve's spam rule function inserted automatically > a custom highlight rule that changed that messages' color, depending > on that header. > > My point is that there is a need for things like my patch and Thierry's > patch, to make this functionality accessible to plugins. > Would anyone mind if I build the plugin architecture for this? I think > it's something like three more hooks needed. Is it that terrible that we > place such hooks in Rfc822Header.class.php? But the proposed patch here does little more than make sure that the message as it is represented inside of SM contains the needed headers as retrieved from IMAP. I think you might be trying to add more functionality than Thierry is with this patch. I think the part of his patch that should be added right away is this: diff -urN squirrelmail/class/mime/Rfc822Header.class.php squirrelmail-melinda/class/mime/Rfc822Header.class.php --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 16:34:17.000000000 +0100 +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 16:09:56.000000000 +0100 @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } This, of course, only adds headers with "X-...." format, so we could change that. I don't see the need for a hook to do that. Adding more hooks means impeding the speed of the core, so if we can avoid that, we should. As for his proposed patch to functions/imap_messages.php, I do not see why that is necessary, because he adds his header to $aMsg['x-priority'], but it *WILL* be added to $aMsg[$field] (which might end up being the same thing?!?!). Again, I think the part of the patch I copied above should be applied now, and the rest can be done with plugins without any further changes to the core. Alexandros, if you are trying to add more complex functionality, we should talk about what exactly that is. |
From: Paul L. <pa...@sq...> - 2008-01-29 22:16:48
|
On Jan 29, 2008 2:15 PM, Paul Lesniewski <pa...@sq...> wrote: > On Jan 24, 2008 3:07 AM, Alexandros Vellis <av...@no...> wrote: > > > > On Thu, 24 Jan 2008 11:45:44 +0100 > > Thijs Kinkhorst <ki...@sq...> wrote: > > > > > Hi Thierry, > > > > > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > > > standard, to be more precise), I needed for a better support of > > > > these headers in Squirrelmail, which for now mostly ignores them. > > > > > > > > The result of this work is the attached tiny patch which: > > > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > > > equivalent to X-Priority, Importance and Priority); > > > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > > > more_headers array, instead of discarding them. > > > > > > Thank you for your patch. I'm a bit reserved about this one, because > > > it adds support for a standard which is specific to the French > > > military. It is only published in French and not through any official > > > standards body. I think this is really the kind of thing that should > > I agree > > > > be implemented in plugins as much as possible. I realise that parsing > > > headers is currently suboptimal from a plugin, but I'd rather see > > > that improved in general than to add such specific headers to our > > > code base... > > Right... > > > I had the exact same issue with an in-house patch of ours, targetted > > towards marking SPAM messages in folders. > > > > What it did was check for X-Spam-<foo> headers in messages inserted > > previously by sendmail, in a patch that is very similar to the one by > > Thierry Godefroy. Avelsieve's spam rule function inserted automatically > > a custom highlight rule that changed that messages' color, depending > > on that header. > > > > My point is that there is a need for things like my patch and Thierry's > > patch, to make this functionality accessible to plugins. > > Would anyone mind if I build the plugin architecture for this? I think > > it's something like three more hooks needed. Is it that terrible that we > > place such hooks in Rfc822Header.class.php? > > But the proposed patch here does little more than make sure that the > message as it is represented inside of SM contains the needed headers > as retrieved from IMAP. I think you might be trying to add more > functionality than Thierry is with this patch. I think the part of > his patch that should be added right away is this: > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > squirrelmail-melinda/class/mime/Rfc822Header.class.php > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > 16:09:56.000000000 +0100 > @@ -353,6 +355,9 @@ > $this->x_spam_status = $this->parseSpamStatus($value); > break; > default: > + if (substr($field, 0, 2) == 'x-') { > + $this->more_headers[$field] = $value; > + } > break; > } > } > > This, of course, only adds headers with "X-...." format, so we could > change that. I don't see the need for a hook to do that. Adding more > hooks means impeding the speed of the core, so if we can avoid that, > we should. > > As for his proposed patch to functions/imap_messages.php, I do not see > why that is necessary, because he adds his header to > $aMsg['x-priority'], but it *WILL* be added to $aMsg[$field] (which > might end up being the same thing?!?!). Sorry, i meant $aMsg['x-p772-primary-precedence'] > Again, I think the part of the patch I copied above should be applied > now, and the rest can be done with plugins without any further changes > to the core. Alexandros, if you are trying to add more complex > functionality, we should talk about what exactly that is. > |
From: Thierry G. <mel...@ya...> - 2008-01-31 15:28:12
Attachments:
squirrelmail-20080128-SVN_devel-MelindaHooks.patch
|
--- Paul Lesniewski <pa...@sq...> wrote: > On Jan 24, 2008 3:07 AM, Alexandros Vellis <av...@no...> wrote: > > > On Thu, 24 Jan 2008 11:45:44 +0100 Thijs Kinkhorst > > <ki...@sq...> wrote: > > > > > Hi Thierry, > > > > > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > > > > > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > > > standard, to be more precise), I needed for a better support of > > > > these headers in Squirrelmail, which for now mostly ignores them. > > > > > > > > The result of this work is the attached tiny patch which: > > > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > > > equivalent to X-Priority, Importance and Priority); > > > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > > > more_headers array, instead of discarding them. > > > > > > Thank you for your patch. I'm a bit reserved about this one, because > > > it adds support for a standard which is specific to the French > > > military. It is only published in French and not through any official > > > standards body. I think this is really the kind of thing that should > > I agree > > > > be implemented in plugins as much as possible. I realise that parsing > > > headers is currently suboptimal from a plugin, but I'd rather see > > > that improved in general than to add such specific headers to our > > > code base... > > Right... > > > I had the exact same issue with an in-house patch of ours, targetted > > towards marking SPAM messages in folders. > > > > What it did was check for X-Spam-<foo> headers in messages inserted > > previously by sendmail, in a patch that is very similar to the one by > > Thierry Godefroy. Avelsieve's spam rule function inserted automatically > > a custom highlight rule that changed that messages' color, depending > > on that header. > > > > My point is that there is a need for things like my patch and Thierry's > > patch, to make this functionality accessible to plugins. > > Would anyone mind if I build the plugin architecture for this? I think > > it's something like three more hooks needed. Is it that terrible that we > > place such hooks in Rfc822Header.class.php? > > But the proposed patch here does little more than make sure that the > message as it is represented inside of SM contains the needed headers > as retrieved from IMAP. I think you might be trying to add more > functionality than Thierry is with this patch. I think the part of > his patch that should be added right away is this: > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > squirrelmail-melinda/class/mime/Rfc822Header.class.php > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > 16:09:56.000000000 +0100 > @@ -353,6 +355,9 @@ > $this->x_spam_status = $this->parseSpamStatus($value); > break; > default: > + if (substr($field, 0, 2) == 'x-') { > + $this->more_headers[$field] = $value; > + } > break; > } > } > > This, of course, only adds headers with "X-...." format, so we could > change that. I don't see the need for a hook to do that. Adding more > hooks means impeding the speed of the core, so if we can avoid that, > we should. > > As for his proposed patch to functions/imap_messages.php, I do not see > why that is necessary, because he adds his header to > $aMsg['x-p772-primary-precedence'], but it *WILL* be added to $aMsg[$field] > (which might end up being the same thing?!?!). Not at all... OK, bear with me, because it's a tad bit intricate... :-P There are two ways the header fields are used: The first one is in mailbox_display.php, where the headers are used to decide what flags should be shown in the message list (and in the case of my plugin, what page the message subject link should point to: either the standard src/read_body.php for normal emails, or my plugin's plugins/melinda/read_body.php, for formal emails). mailbox_display.php uses $aMsg['x-priority'] to decide if the message should be flagged as urgent in the list, and $aMsg['x-priority'] was set by parseFetch() (in functions/imap_messages.php) after retrieving a _short_ list of headers with sqimap_get_small_header_list() (still in functions/imap_messages.php), among which the X-Priority, Priority and Importance fields. Equally, my plugin (which is called, _after_ the priority flag was set, thus the need for the patch adding x-p772-primary-precedence in parseFetch()), uses the other X-SMTP headers it made mailbox_display.php fetch (via the new 'fetch_more_headers' hook) to decide if the email subject link should be changed to point at its own version of read_body.php (thus the need for allowing to change this link via the 'subject_link' hook, again a small change part of my 'MelindaHooks' patch). During the mailbox displaying process, all the messages got their headers fetched from the IMAP server, thus the need for fetching only a minimal amount of headers for each message (and indeed, in its latest version, my plugin now only fetches two extra X-P772 headers when compared to the normal version of SQM). The second use of the headers is for displaying the messages (via read_body.php) and composing replies or forwarding them (via compose.php): here we only fetch the headers of one message, but we do need to fetch them all (thus the need for the X-SMTP headers patch in class/mime/Rfc822Header.class.php). I hope this clears it all... :-) I attached the latest MelindaHooks patch to this message: it was a patch against the 2008/01/28 version of SQM devel, so there might be rejects with the latest sources if you already adopted some of my patches. This new version adds the new 'next_or_previous_link' hook (which was already used in my previous patch for read_body.php) to the pages responsible for displaying the attachments, as we need the "View message" link to point back to the right page (in my case, either the normal read_body.php, or Melinda's one). The patched files are: src/image.php, src/view_text.php and src/vcard.php. As the mailbox cache is not readily available in these cases, I needed to change a bit the meaning of the first parameter passed in the 'next_or_previous_link' hook: it's now either an array (the cached mailbox), or it is a string (the mailbox name). Here is how to use that hook from a plugin: /* * Adjust the link for next or previous messages in the read_body menu bar, * and the view message link for attachments viewing pages. */ function myplugin_next_or_previous_link_do(&$argv) { $mailbox = $argv[0]; $msg = $argv[1]; $link = &$argv[2]; if (!is_array($mailbox)) { // $mailbox is only the name of the mailbox, and not the $aMailbox // array (this is the case when this hook is called from the // attachments viewing pages). So, let's retrieve that array. sqgetGlobalVar('mailbox_cache', $mailbox_cache, SQ_SESSION); if (sqgetGlobalVar('account', $temp, SQ_GET)) { $account = (int)$temp; } else { $account = 0; } if (isset($mailbox_cache[$account . '_' . $mailbox])) { $mailbox = $mailbox_cache[$account . '_' . $mailbox]; } else { // Mailbox cache not found (should never happen) ! // Give up instead of doing stupid things... return; } } if (isset($mailbox['MSG_HEADERS'][$msg])) { if (myplugin_is_it_my_stuff($mailbox['MSG_HEADERS'][$msg])) { if (strpos($link, 'read_body.php') == 0) { $link = '../plugins/myplugin/' . $link; } else { $link = str_replace('/src/', '/plugins/myplugin/', $link); } } else { $link = str_replace('/plugins/myplugin/', '/src/', $link); } } } where myplugin_is_it_my_stuff() is a function deciding if the message is to be displayed by the plugin or not, based on the array of headers it is passed. Regards, Thierry Godefroy. ____________________________________________________________________________________ Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping |
From: Paul L. <pa...@sq...> - 2008-02-03 23:46:21
|
On Jan 31, 2008 7:28 AM, Thierry Godefroy <mel...@ya...> wrote: > > --- Paul Lesniewski <pa...@sq...> wrote: > > > On Jan 24, 2008 3:07 AM, Alexandros Vellis <av...@no...> wrote: > > > > > On Thu, 24 Jan 2008 11:45:44 +0100 Thijs Kinkhorst > > > <ki...@sq...> wrote: > > > > > > > Hi Thierry, > > > > > > > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > > > > > > > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > > > > standard, to be more precise), I needed for a better support of > > > > > these headers in Squirrelmail, which for now mostly ignores them. > > > > > > > > > > The result of this work is the attached tiny patch which: > > > > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > > > > equivalent to X-Priority, Importance and Priority); > > > > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > > > > more_headers array, instead of discarding them. > > > > > > > > Thank you for your patch. I'm a bit reserved about this one, because > > > > it adds support for a standard which is specific to the French > > > > military. It is only published in French and not through any official > > > > standards body. I think this is really the kind of thing that should > > > > I agree > > > > > > be implemented in plugins as much as possible. I realise that parsing > > > > headers is currently suboptimal from a plugin, but I'd rather see > > > > that improved in general than to add such specific headers to our > > > > code base... > > > > Right... > > > > > I had the exact same issue with an in-house patch of ours, targetted > > > towards marking SPAM messages in folders. > > > > > > What it did was check for X-Spam-<foo> headers in messages inserted > > > previously by sendmail, in a patch that is very similar to the one by > > > Thierry Godefroy. Avelsieve's spam rule function inserted automatically > > > a custom highlight rule that changed that messages' color, depending > > > on that header. > > > > > > My point is that there is a need for things like my patch and Thierry's > > > patch, to make this functionality accessible to plugins. > > > Would anyone mind if I build the plugin architecture for this? I think > > > it's something like three more hooks needed. Is it that terrible that we > > > place such hooks in Rfc822Header.class.php? > > > > But the proposed patch here does little more than make sure that the > > message as it is represented inside of SM contains the needed headers > > as retrieved from IMAP. I think you might be trying to add more > > functionality than Thierry is with this patch. I think the part of > > his patch that should be added right away is this: > > > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > > squirrelmail-melinda/class/mime/Rfc822Header.class.php > > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > > 16:34:17.000000000 +0100 > > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > > 16:09:56.000000000 +0100 > > @@ -353,6 +355,9 @@ > > $this->x_spam_status = $this->parseSpamStatus($value); > > break; > > default: > > + if (substr($field, 0, 2) == 'x-') { > > + $this->more_headers[$field] = $value; > > + } > > break; > > } > > } > > > > This, of course, only adds headers with "X-...." format, so we could > > change that. I don't see the need for a hook to do that. Adding more > > hooks means impeding the speed of the core, so if we can avoid that, > > we should. > > > > As for his proposed patch to functions/imap_messages.php, I do not see > > why that is necessary, because he adds his header to > > $aMsg['x-p772-primary-precedence'], but it *WILL* be added to $aMsg[$field] > > (which might end up being the same thing?!?!). > > Not at all... OK, bear with me, because it's a tad bit intricate... :-P > > There are two ways the header fields are used: > > The first one is in mailbox_display.php, where the headers are used to decide > what flags should be shown in the message list (and in the case of my plugin, > what page the message subject link should point to: either the standard > src/read_body.php for normal emails, or my plugin's > plugins/melinda/read_body.php, for formal emails). > mailbox_display.php uses $aMsg['x-priority'] to decide if the message should be > flagged as urgent in the list, and $aMsg['x-priority'] was set by parseFetch() > (in functions/imap_messages.php) after retrieving a _short_ list of headers > with sqimap_get_small_header_list() (still in functions/imap_messages.php), > among which the X-Priority, Priority and Importance fields. Equally, my plugin > (which is called, _after_ the priority flag was set, thus the need for the > patch adding x-p772-primary-precedence in parseFetch()), uses the other X-SMTP > headers it made mailbox_display.php fetch (via the new 'fetch_more_headers' > hook) to decide if the email subject link should be changed to point at its own > version of read_body.php (thus the need for allowing to change this link via > the 'subject_link' hook, again a small change part of my 'MelindaHooks' patch). > During the mailbox displaying process, all the messages got their headers > fetched from the IMAP server, thus the need for fetching only a minimal amount > of headers for each message (and indeed, in its latest version, my plugin now > only fetches two extra X-P772 headers when compared to the normal version of > SQM). > > The second use of the headers is for displaying the messages (via > read_body.php) and composing replies or forwarding them (via compose.php): here > we only fetch the headers of one message, but we do need to fetch them all > (thus the need for the X-SMTP headers patch in > class/mime/Rfc822Header.class.php). > > I hope this clears it all... :-) Well, no, this does not explain why my comment above is wrong. You introduced some different patches in this email, but for now, I am addressing the ones you started this thread with. First, this one, which I think is not needed: diff -urN squirrelmail/functions/imap_messages.php squirrelmail-melinda/functions/imap_messages.php --- squirrelmail/functions/imap_messages.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 11:09:20.000000000 +0100 @@ -700,6 +700,8 @@ $aMsg['date'] = trim(str_replace(' ', ' ', $value)); break; case 'x-priority': $aMsg['x-priority'] = ($value) ? (int) $value{0} : 3; break; + case 'x-p772-primary-precedence': + $aMsg['x-p772-primary-precedence'] = $value; case 'priority': case 'importance': // duplicate code with Rfc822Header.cls:parsePriority() This is not needed because the default case just below looks like this: default: $aMsg[$field] = $value; break; So in the end, $aMsg has the very same value added to it. Now what I am seeing that may be key to your patch is that x-p772-primary-precedence does not have a "break;" so it also triggers the execution of the code block that sets the priority ($aMsg['x-priority']). Are you claiming that at the time that you are using this code that you need that block to be executed? I do not like adding more headers to this switch block. Do you think you could fetch the importance, priority, or x-priority header(s) instead to trigger this block's execution? The second patch that I don't think is needed: diff -urN squirrelmail/class/mime/Rfc822Header.class.php squirrelmail-melinda/class/mime/Rfc822Header.class.php --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 16:34:17.000000000 +0100 +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 16:09:56.000000000 +0100 @@ -312,6 +312,8 @@ case 'x-mailer': $this->xmailer = $value; break; + case 'x-p772-primary-precedence': + $this->more_headers[$field] = $value; case 'x-priority': case 'importance': case 'priority': This is not needed for the same reason - because I think the one part of your patch that SHOULD be added will result in $this->more_headers having the very same information added to it anyway: @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } Here, too, I think I see that you are trying to get the priority code block to trigger when your header is seen too. Again, I'd rather see this accomplished by possibly fetching the standard priority fields (or faking them by adding them to the message object yourself). So again, I think these are good (but Alexandros might have something a little different): diff -urN squirrelmail/class/mime/Rfc822Header.class.php squirrelmail-melinda/class/mime/Rfc822Header.class.php --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 16:34:17.000000000 +0100 +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 16:09:56.000000000 +0100 @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } diff -urN squirrelmail/functions/mailbox_display.php squirrelmail-patched/functions/mailbox_display.php --- squirrelmail/functions/mailbox_display.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 10:01:37.000000000 +0100 @@ -315,6 +315,9 @@ } } + // For X-SMTP headers we could need to fetch: + do_hook('fetch_more_headers', &$aHeaderFields); + /** * A uidset with sorted uid's is available. We can use the cache */ I'd rather see if the other parts can be addressed in a different manner. As for the other parts of your patch that you added in your last message on this thread, I'd rather address them in a different thread or different message on this thread once we resolve these. Thank you! > I attached the latest MelindaHooks patch to this message: it was a patch > against the 2008/01/28 version of SQM devel, so there might be rejects with the > latest sources if you already adopted some of my patches. > > This new version adds the new 'next_or_previous_link' hook (which was already > used in my previous patch for read_body.php) to the pages responsible for > displaying the attachments, as we need the "View message" link to point back to > the right page (in my case, either the normal read_body.php, or Melinda's one). > The patched files are: src/image.php, src/view_text.php and src/vcard.php. As > the mailbox cache is not readily available in these cases, I needed to change a > bit the meaning of the first parameter passed in the 'next_or_previous_link' > hook: it's now either an array (the cached mailbox), or it is a string (the > mailbox name). Here is how to use that hook from a plugin: > > /* > * Adjust the link for next or previous messages in the read_body menu bar, > * and the view message link for attachments viewing pages. > */ > function myplugin_next_or_previous_link_do(&$argv) { > $mailbox = $argv[0]; > $msg = $argv[1]; > $link = &$argv[2]; > > if (!is_array($mailbox)) { > // $mailbox is only the name of the mailbox, and not the $aMailbox > // array (this is the case when this hook is called from the > // attachments viewing pages). So, let's retrieve that array. > sqgetGlobalVar('mailbox_cache', $mailbox_cache, SQ_SESSION); > if (sqgetGlobalVar('account', $temp, SQ_GET)) { > $account = (int)$temp; > } else { > $account = 0; > } > if (isset($mailbox_cache[$account . '_' . $mailbox])) { > $mailbox = $mailbox_cache[$account . '_' . $mailbox]; > } else { > // Mailbox cache not found (should never happen) ! > // Give up instead of doing stupid things... > return; > } > } > > if (isset($mailbox['MSG_HEADERS'][$msg])) { > if (myplugin_is_it_my_stuff($mailbox['MSG_HEADERS'][$msg])) { > if (strpos($link, 'read_body.php') == 0) { > $link = '../plugins/myplugin/' . $link; > } else { > $link = str_replace('/src/', '/plugins/myplugin/', $link); > } > } else { > $link = str_replace('/plugins/myplugin/', '/src/', $link); > } > } > } > > where myplugin_is_it_my_stuff() is a function deciding if the message is to be > displayed by the plugin or not, based on the array of headers it is passed. |
From: Paul L. <pa...@sq...> - 2008-02-03 23:53:46
|
On Feb 3, 2008 3:46 PM, Paul Lesniewski <pa...@sq...> wrote: > > On Jan 31, 2008 7:28 AM, Thierry Godefroy <mel...@ya...> wrote: > > > > --- Paul Lesniewski <pa...@sq...> wrote: > > > > > On Jan 24, 2008 3:07 AM, Alexandros Vellis <av...@no...> wrote: > > > > > > > On Thu, 24 Jan 2008 11:45:44 +0100 Thijs Kinkhorst > > > > <ki...@sq...> wrote: > > > > > > > > > Hi Thierry, > > > > > > > > > > On Thursday 24 January 2008 11:07, Thierry Godefroy wrote: > > > > > > > > > > > As I am developing a plugin which relies on X-SMTP headers (X-P772 > > > > > > standard, to be more precise), I needed for a better support of > > > > > > these headers in Squirrelmail, which for now mostly ignores them. > > > > > > > > > > > > The result of this work is the attached tiny patch which: > > > > > > - adds X-P772-Primary-Precedence support (this is the X-P772 > > > > > > equivalent to X-Priority, Importance and Priority); > > > > > > - makes Rfc822Header.class store any X-SMTP header it parses in its > > > > > > more_headers array, instead of discarding them. > > > > > > > > > > Thank you for your patch. I'm a bit reserved about this one, because > > > > > it adds support for a standard which is specific to the French > > > > > military. It is only published in French and not through any official > > > > > standards body. I think this is really the kind of thing that should > > > > > > I agree > > > > > > > > be implemented in plugins as much as possible. I realise that parsing > > > > > headers is currently suboptimal from a plugin, but I'd rather see > > > > > that improved in general than to add such specific headers to our > > > > > code base... > > > > > > Right... > > > > > > > I had the exact same issue with an in-house patch of ours, targetted > > > > towards marking SPAM messages in folders. > > > > > > > > What it did was check for X-Spam-<foo> headers in messages inserted > > > > previously by sendmail, in a patch that is very similar to the one by > > > > Thierry Godefroy. Avelsieve's spam rule function inserted automatically > > > > a custom highlight rule that changed that messages' color, depending > > > > on that header. > > > > > > > > My point is that there is a need for things like my patch and Thierry's > > > > patch, to make this functionality accessible to plugins. > > > > Would anyone mind if I build the plugin architecture for this? I think > > > > it's something like three more hooks needed. Is it that terrible that we > > > > place such hooks in Rfc822Header.class.php? > > > > > > But the proposed patch here does little more than make sure that the > > > message as it is represented inside of SM contains the needed headers > > > as retrieved from IMAP. I think you might be trying to add more > > > functionality than Thierry is with this patch. I think the part of > > > his patch that should be added right away is this: > > > > > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > > > squirrelmail-melinda/class/mime/Rfc822Header.class.php > > > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > > > 16:34:17.000000000 +0100 > > > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > > > 16:09:56.000000000 +0100 > > > @@ -353,6 +355,9 @@ > > > $this->x_spam_status = $this->parseSpamStatus($value); > > > break; > > > default: > > > + if (substr($field, 0, 2) == 'x-') { > > > + $this->more_headers[$field] = $value; > > > + } > > > break; > > > } > > > } > > > > > > This, of course, only adds headers with "X-...." format, so we could > > > change that. I don't see the need for a hook to do that. Adding more > > > hooks means impeding the speed of the core, so if we can avoid that, > > > we should. > > > > > > As for his proposed patch to functions/imap_messages.php, I do not see > > > why that is necessary, because he adds his header to > > > $aMsg['x-p772-primary-precedence'], but it *WILL* be added to $aMsg[$field] > > > (which might end up being the same thing?!?!). > > > > Not at all... OK, bear with me, because it's a tad bit intricate... :-P > > > > There are two ways the header fields are used: > > > > The first one is in mailbox_display.php, where the headers are used to decide > > what flags should be shown in the message list (and in the case of my plugin, > > what page the message subject link should point to: either the standard > > src/read_body.php for normal emails, or my plugin's > > plugins/melinda/read_body.php, for formal emails). > > mailbox_display.php uses $aMsg['x-priority'] to decide if the message should be > > flagged as urgent in the list, and $aMsg['x-priority'] was set by parseFetch() > > (in functions/imap_messages.php) after retrieving a _short_ list of headers > > with sqimap_get_small_header_list() (still in functions/imap_messages.php), > > among which the X-Priority, Priority and Importance fields. Equally, my plugin > > (which is called, _after_ the priority flag was set, thus the need for the > > patch adding x-p772-primary-precedence in parseFetch()), uses the other X-SMTP > > headers it made mailbox_display.php fetch (via the new 'fetch_more_headers' > > hook) to decide if the email subject link should be changed to point at its own > > version of read_body.php (thus the need for allowing to change this link via > > the 'subject_link' hook, again a small change part of my 'MelindaHooks' patch). > > During the mailbox displaying process, all the messages got their headers > > fetched from the IMAP server, thus the need for fetching only a minimal amount > > of headers for each message (and indeed, in its latest version, my plugin now > > only fetches two extra X-P772 headers when compared to the normal version of > > SQM). > > > > The second use of the headers is for displaying the messages (via > > read_body.php) and composing replies or forwarding them (via compose.php): here > > we only fetch the headers of one message, but we do need to fetch them all > > (thus the need for the X-SMTP headers patch in > > class/mime/Rfc822Header.class.php). > > > > I hope this clears it all... :-) > > Well, no, this does not explain why my comment above is wrong. You > introduced some different patches in this email, but for now, I am > addressing the ones you started this thread with. First, this one, > which I think is not needed: > > diff -urN squirrelmail/functions/imap_messages.php > squirrelmail-melinda/functions/imap_messages.php > --- squirrelmail/functions/imap_messages.php 2008-01-15 16:34:31.000000000 +0100 > +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 > 11:09:20.000000000 +0100 > @@ -700,6 +700,8 @@ > $aMsg['date'] = > trim(str_replace(' ', ' ', $value)); > break; > case 'x-priority': > $aMsg['x-priority'] = ($value) ? (int) $value{0} : 3; break; > + case 'x-p772-primary-precedence': > + > $aMsg['x-p772-primary-precedence'] = $value; > case 'priority': > case 'importance': > // duplicate code with > Rfc822Header.cls:parsePriority() > > This is not needed because the default case just below looks like this: > > default: > $aMsg[$field] = $value; > break; > > So in the end, $aMsg has the very same value added to it. Now what I > am seeing that may be key to your patch is that > x-p772-primary-precedence does not have a "break;" so it also triggers > the execution of the code block that sets the priority > ($aMsg['x-priority']). Are you claiming that at the time that you are > using this code that you need that block to be executed? I do not > like adding more headers to this switch block. Do you think you could > fetch the importance, priority, or x-priority header(s) instead to > trigger this block's execution? > > The second patch that I don't think is needed: > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > squirrelmail-melinda/class/mime/Rfc822Header.class.php > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > 16:09:56.000000000 +0100 > @@ -312,6 +312,8 @@ > case 'x-mailer': > $this->xmailer = $value; > break; > + case 'x-p772-primary-precedence': > + $this->more_headers[$field] = $value; > case 'x-priority': > case 'importance': > case 'priority': > > This is not needed for the same reason - because I think the one part > of your patch that SHOULD be added will result in $this->more_headers > having the very same information added to it anyway: > > @@ -353,6 +355,9 @@ > $this->x_spam_status = $this->parseSpamStatus($value); > break; > default: > + if (substr($field, 0, 2) == 'x-') { > + $this->more_headers[$field] = $value; > + } > break; > } > } > > Here, too, I think I see that you are trying to get the priority code > block to trigger when your header is seen too. Again, I'd rather see > this accomplished by possibly fetching the standard priority fields > (or faking them by adding them to the message object yourself). > > So again, I think these are good (but Alexandros might have something > a little different): > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > squirrelmail-melinda/class/mime/Rfc822Header.class.php > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > 16:09:56.000000000 +0100 > @@ -353,6 +355,9 @@ > $this->x_spam_status = $this->parseSpamStatus($value); > break; > default: > + if (substr($field, 0, 2) == 'x-') { > + $this->more_headers[$field] = $value; > + } > break; > } > } > diff -urN squirrelmail/functions/mailbox_display.php > squirrelmail-patched/functions/mailbox_display.php > --- squirrelmail/functions/mailbox_display.php 2008-01-15 > 16:34:31.000000000 +0100 > +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 > 10:01:37.000000000 +0100 > @@ -315,6 +315,9 @@ > } > } > > + // For X-SMTP headers we could need to fetch: > + do_hook('fetch_more_headers', &$aHeaderFields); > + > /** > * A uidset with sorted uid's is available. We can use the cache > */ Sorry, this should also have been part of the changes I personally agree with: diff -urN squirrelmail/functions/mailbox_display.php squirrelmail-patched/functions/mailbox_display.php --- squirrelmail/functions/mailbox_display.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 10:01:37.000000000 +0100 @@ -315,6 +315,9 @@ } } + // For X-SMTP headers we could need to fetch: + do_hook('fetch_more_headers', &$aHeaderFields); + /** * A uidset with sorted uid's is available. We can use the cache */ > I'd rather see if the other parts can be addressed in a different manner. > > As for the other parts of your patch that you added in your last > message on this thread, I'd rather address them in a different thread > or different message on this thread once we resolve these. > > Thank you! > > > > I attached the latest MelindaHooks patch to this message: it was a patch > > against the 2008/01/28 version of SQM devel, so there might be rejects with the > > latest sources if you already adopted some of my patches. > > > > This new version adds the new 'next_or_previous_link' hook (which was already > > used in my previous patch for read_body.php) to the pages responsible for > > displaying the attachments, as we need the "View message" link to point back to > > the right page (in my case, either the normal read_body.php, or Melinda's one). > > The patched files are: src/image.php, src/view_text.php and src/vcard.php. As > > the mailbox cache is not readily available in these cases, I needed to change a > > bit the meaning of the first parameter passed in the 'next_or_previous_link' > > hook: it's now either an array (the cached mailbox), or it is a string (the > > mailbox name). Here is how to use that hook from a plugin: > > > > /* > > * Adjust the link for next or previous messages in the read_body menu bar, > > * and the view message link for attachments viewing pages. > > */ > > function myplugin_next_or_previous_link_do(&$argv) { > > $mailbox = $argv[0]; > > $msg = $argv[1]; > > $link = &$argv[2]; > > > > if (!is_array($mailbox)) { > > // $mailbox is only the name of the mailbox, and not the $aMailbox > > // array (this is the case when this hook is called from the > > // attachments viewing pages). So, let's retrieve that array. > > sqgetGlobalVar('mailbox_cache', $mailbox_cache, SQ_SESSION); > > if (sqgetGlobalVar('account', $temp, SQ_GET)) { > > $account = (int)$temp; > > } else { > > $account = 0; > > } > > if (isset($mailbox_cache[$account . '_' . $mailbox])) { > > $mailbox = $mailbox_cache[$account . '_' . $mailbox]; > > } else { > > // Mailbox cache not found (should never happen) ! > > // Give up instead of doing stupid things... > > return; > > } > > } > > > > if (isset($mailbox['MSG_HEADERS'][$msg])) { > > if (myplugin_is_it_my_stuff($mailbox['MSG_HEADERS'][$msg])) { > > if (strpos($link, 'read_body.php') == 0) { > > $link = '../plugins/myplugin/' . $link; > > } else { > > $link = str_replace('/src/', '/plugins/myplugin/', $link); > > } > > } else { > > $link = str_replace('/plugins/myplugin/', '/src/', $link); > > } > > } > > } > > > > where myplugin_is_it_my_stuff() is a function deciding if the message is to be > > displayed by the plugin or not, based on the array of headers it is passed. > |
From: Thierry G. <mel...@ya...> - 2008-02-04 09:36:15
|
--- Paul Lesniewski <pa...@sq...> wrote: > On Jan 31, 2008 7:28 AM, Thierry Godefroy <mel...@ya...> wrote: > > > > .../... > > > > I hope this clears it all... :-) > > Well, no, this does not explain why my comment above is wrong. You > introduced some different patches in this email, but for now, I am > addressing the ones you started this thread with. First, this one, > which I think is not needed: > > diff -urN squirrelmail/functions/imap_messages.php > squirrelmail-melinda/functions/imap_messages.php > --- squirrelmail/functions/imap_messages.php 2008-01-15 16:34:31.000000000 > +0100 > +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 > 11:09:20.000000000 +0100 > @@ -700,6 +700,8 @@ > $aMsg['date'] = > trim(str_replace(' ', ' ', $value)); > break; > case 'x-priority': > $aMsg['x-priority'] = ($value) ? (int) $value{0} : 3; break; > + case 'x-p772-primary-precedence': > + > $aMsg['x-p772-primary-precedence'] = $value; > case 'priority': > case 'importance': > // duplicate code with > Rfc822Header.cls:parsePriority() > > This is not needed because the default case just below looks like this: > > default: > $aMsg[$field] = $value; > break; > > So in the end, $aMsg has the very same value added to it. Now what I > am seeing that may be key to your patch is that > x-p772-primary-precedence does not have a "break;" so it also triggers > the execution of the code block that sets the priority > ($aMsg['x-priority']). Are you claiming that at the time that you are > using this code that you need that block to be executed? Of course yes ! Please, do read carefully the explanations I gave into my email: I need this so that mailbox_display.php does set properly the priority flag in the message list: because this flag is parsed/set at a "random" time (in fact, it depends on the order configured by the user for the messages list fields), I cannot rely on the only available hook in prepareMessageList() (functions/mailbox_display.php), which is the subject_link hook, to adjust the priority value before the SQM_COL_PRIO case is executed. > I do not like adding more headers to this switch block. The alternative is to add a hook in prepareMessageList(), for the SQM_COL_PRIO case... A bit of an overkill (in term of required processing power) when compared to my patch (two lines of code added)... > Do you think you could fetch the importance, priority, or x-priority > header(s) instead to trigger this block's execution? No, because those fields are not used (and they are always absent) in formal messages. Formal messages make a distinction between primary (for 'To' adressees) and copy (for 'Info' addressees = 'Cc') precedences. > The second patch that I don't think is needed: > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > squirrelmail-melinda/class/mime/Rfc822Header.class.php > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > 16:09:56.000000000 +0100 > @@ -312,6 +312,8 @@ > case 'x-mailer': > $this->xmailer = $value; > break; > + case 'x-p772-primary-precedence': > + $this->more_headers[$field] = $value; > case 'x-priority': > case 'importance': > case 'priority': > > This is not needed for the same reason - because I think the one part > of your patch that SHOULD be added will result in $this->more_headers > having the very same information added to it anyway: But again, if you remove this patch, the priority of the message (in the header array) will not be properly set for formal messages... There -is- a reason for the absence of a break, as I do need the code in 'priority' case to be executed. Here again, the only alternative is to add a hook somewhere, excepted that this class being called in various places, this would involve adding a hook to the class itself (in the default: case)... I again see this as a waste of processing power, really... Another possibility, which I suggested already in a previous reply to your objections, would be to make the priority field names configurable: for example by putting 'X-Priority' (which is already an extension to the RFC-822 anyway) into an array and processing all fields in that array in the same way: adding more fields to this array (such as X-P772-Primary-Precedence) should then be made possible via a site-wide configuration option (in config/config_default.php, for example)... Regards, Thierry Godefroy. ____________________________________________________________________________________ Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ |
From: Paul L. <pa...@sq...> - 2008-02-05 03:35:20
|
On 2/4/08, Thierry Godefroy <mel...@ya...> wrote: > --- Paul Lesniewski <pa...@sq...> wrote: > > > On Jan 31, 2008 7:28 AM, Thierry Godefroy <mel...@ya...> wrote: > > > > > > .../... > > > > > > I hope this clears it all... :-) > > > > Well, no, this does not explain why my comment above is wrong. You > > introduced some different patches in this email, but for now, I am > > addressing the ones you started this thread with. First, this one, > > which I think is not needed: > > > > diff -urN squirrelmail/functions/imap_messages.php > > squirrelmail-melinda/functions/imap_messages.php > > --- squirrelmail/functions/imap_messages.php 2008-01-15 16:34:31.000000000 > > +0100 > > +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 > > 11:09:20.000000000 +0100 > > @@ -700,6 +700,8 @@ > > $aMsg['date'] = > > trim(str_replace(' ', ' ', $value)); > > break; > > case 'x-priority': > > $aMsg['x-priority'] = ($value) ? (int) $value{0} : 3; break; > > + case 'x-p772-primary-precedence': > > + > > $aMsg['x-p772-primary-precedence'] = $value; > > case 'priority': > > case 'importance': > > // duplicate code with > > Rfc822Header.cls:parsePriority() > > > > This is not needed because the default case just below looks like this: > > > > default: > > $aMsg[$field] = $value; > > break; > > > > So in the end, $aMsg has the very same value added to it. Now what I > > am seeing that may be key to your patch is that > > x-p772-primary-precedence does not have a "break;" so it also triggers > > the execution of the code block that sets the priority > > ($aMsg['x-priority']). Are you claiming that at the time that you are > > using this code that you need that block to be executed? > > Of course yes ! > > Please, do read carefully the explanations I gave into my email: I need this so > that mailbox_display.php does set properly the priority flag in the message > list: because this flag is parsed/set at a "random" time (in fact, it depends > on the order configured by the user for the messages list fields), I cannot > rely on the only available hook in prepareMessageList() > (functions/mailbox_display.php), which is the subject_link hook, to adjust the > priority value before the SQM_COL_PRIO case is executed. > > > I do not like adding more headers to this switch block. > > The alternative is to add a hook in prepareMessageList(), for the SQM_COL_PRIO > case... A bit of an overkill (in term of required processing power) when > compared to my patch (two lines of code added)... > > > Do you think you could fetch the importance, priority, or x-priority > > header(s) instead to trigger this block's execution? > > No, because those fields are not used (and they are always absent) in formal > messages. Formal messages make a distinction between primary (for 'To' > adressees) and copy (for 'Info' addressees = 'Cc') precedences. I'm not sure if there is a good hook to do it with before all of this mess, but I don't think there is any harm in injecting a x-priority field with a copy of whatever is in x-p772-primary-precedence. That should then trigger the correct code to run. > > The second patch that I don't think is needed: > > > > diff -urN squirrelmail/class/mime/Rfc822Header.class.php > > squirrelmail-melinda/class/mime/Rfc822Header.class.php > > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > > 16:34:17.000000000 +0100 > > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 > > 16:09:56.000000000 +0100 > > @@ -312,6 +312,8 @@ > > case 'x-mailer': > > $this->xmailer = $value; > > break; > > + case 'x-p772-primary-precedence': > > + $this->more_headers[$field] = $value; > > case 'x-priority': > > case 'importance': > > case 'priority': > > > > This is not needed for the same reason - because I think the one part > > of your patch that SHOULD be added will result in $this->more_headers > > having the very same information added to it anyway: > > But again, if you remove this patch, the priority of the message (in the header > array) will not be properly set for formal messages... There -is- a reason for > the absence of a break, as I do need the code in 'priority' case to be > executed. Here again, the only alternative is to add a hook somewhere, excepted > that this class being called in various places, this would involve adding a > hook to the class itself (in the default: case)... I again see this as a waste > of processing power, really... > > Another possibility, which I suggested already in a previous reply to your > objections, would be to make the priority field names configurable: for example > by putting 'X-Priority' (which is already an extension to the RFC-822 anyway) > into an array and processing all fields in that array in the same way: adding > more fields to this array (such as X-P772-Primary-Precedence) should then be > made possible via a site-wide configuration option (in > config/config_default.php, for example)... I'm not sure I caught that, but it's not a horrible idea. At this point, however, if you can't spoof a X-Priority field as I suggested, then I am inclined to reconsider this part of your patch. Not sure what Thijs has to say about this.... but I think that even if the X-P772-Primary-Precedence header was invented by some guy in a cabin in the woods, if we don't have a way to get this working for you, I'm amenable to adding it to the core. |
From: Thierry G. <mel...@ya...> - 2008-02-05 09:25:12
Attachments:
squirrelmail-20080201-SVN_devel-XSMTP_V2.patch
|
--- Paul Lesniewski <pa...@sq...> wrote: > On 2/4/08, Thierry Godefroy <mel...@ya...> wrote: > > > --- Paul Lesniewski <pa...@sq...> wrote: > > > > > On Jan 31, 2008 7:28 AM, Thierry Godefroy <mel...@ya...> > > > wrote: > > > > > > > .../... > > > > > > Do you think you could fetch the importance, priority, or x-priority > > > header(s) instead to trigger this block's execution? > > > > No, because those fields are not used (and they are always absent) in > > formal messages. Formal messages make a distinction between primary (for > > 'To' adressees) and copy (for 'Info' addressees = 'Cc') precedences. > > I'm not sure if there is a good hook to do it with before all of this > mess, but I don't think there is any harm in injecting a x-priority > field with a copy of whatever is in x-p772-primary-precedence. That > should then trigger the correct code to run. There is no hook that I am aware of which would be called soon enough (and after the headers were fetched) to perform successfully such an injection... > > .../... Zap ! > > > > Another possibility, which I suggested already in a previous reply to your > > objections, would be to make the priority field names configurable: for > > example by putting 'X-Priority' (which is already an extension to the > > RFC-822 anyway) into an array and processing all fields in that array in > > the same way: adding more fields to this array (such as > > X-P772-Primary-Precedence) should then be > > made possible via a site-wide configuration option (in > > config/config_default.php, for example)... > > I'm not sure I caught that, but it's not a horrible idea. At this > point, however, if you can't spoof a X-Priority field as I suggested, > then I am inclined to reconsider this part of your patch. Not sure > what Thijs has to say about this.... but I think that even if the > X-P772-Primary-Precedence header was invented by some guy in a cabin > in the woods, if we don't have a way to get this working for you, I'm > amenable to adding it to the core. Well, I generalized the idea to all Importance/Priority/Precedence fields and implemented it. See the attached patch. ;-) Regards, Thierry. ____________________________________________________________________________________ Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs |
From: Alexandros V. <av...@no...> - 2008-02-06 10:16:10
|
[/Meta/: uhm, sorry Paul but all this quoting business confuses me. :) I've grown accustomed to cut most quoted lines except for one or two, that just help provide the context of the answer.] Anyway, I only have one remark about the patch in functions/imap_messages.php, that deals with extra headers to fetch in message list: I'd rather have something like: Index: imap_messages.php =================================================================== --- imap_messages.php (revision 12922) +++ imap_messages.php (working copy) @@ -560,6 +560,9 @@ } $bUidFetch = ! in_array('UID', $aFetchItems, true); + + $temp = array(&$aHeaderFields, &$aFetchItems); + do_hook('small_header_list', $temp); /* Get the small headers for each message in $msg_list */ if ($msg_list !== NULL) { --------------------- ['small_header_list' is my proposal for this hook name] Yes, pass $aHeaderFields and _in addition_ $aFetchItems through to a plugin. Why? Because I also have an IMAP ANNOTATE plugin in mind. 8-) (One can get message - not folder - annotations via an extra FETCH item). That's about it. As for the rest, I don't know about the priority headers... But as long as the plugin hooks are generalized enough to allow me to fetch ANY header in my _plugin_, in message display screen (read_body), then I'll be satisfied. Alexandros |
From: Paul L. <pa...@sq...> - 2008-02-07 06:39:31
|
On 2/6/08, Alexandros Vellis <av...@no...> wrote: > [/Meta/: uhm, sorry Paul but all this quoting business confuses me. :) > I've grown accustomed to cut most quoted lines except for one or two, > that just help provide the context of the answer.] > > Anyway, I only have one remark about the patch in > functions/imap_messages.php, that deals with extra headers to fetch in > message list: > > I'd rather have something like: > > Index: imap_messages.php > =================================================================== > --- imap_messages.php (revision 12922) > +++ imap_messages.php (working copy) > @@ -560,6 +560,9 @@ > } > > $bUidFetch = ! in_array('UID', $aFetchItems, true); > + > + $temp = array(&$aHeaderFields, &$aFetchItems); > + do_hook('small_header_list', $temp); > > /* Get the small headers for each message in $msg_list */ > if ($msg_list !== NULL) { > > > --------------------- > ['small_header_list' is my proposal for this hook name] > > Yes, pass $aHeaderFields and _in addition_ $aFetchItems through to a > plugin. Why? Because I also have an IMAP ANNOTATE plugin in mind. 8-) > (One can get message - not folder - annotations via an extra FETCH > item). That makes sense. So the following part of the proposed patch is what this replaces, correct? --- squirrelmail/functions/mailbox_display.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-patched/functions/mailbox_display.php 2008-01-24 10:01:37.000000000 +0100 @@ -315,6 +315,9 @@ } } + // For X-SMTP headers we could need to fetch: + do_hook('fetch_more_headers', &$aHeaderFields); + /** * A uidset with sorted uid's is available. We can use the cache */ > That's about it. As for the rest, I don't know about the priority > headers... But as long as the plugin hooks are generalized enough to > allow me to fetch ANY header in my _plugin_, in message display screen > (read_body), then I'll be satisfied. So do you agree with the following? Thierry, how about you? Does this get everything (except the other next/previous links changes you sent on this thread, which I'd like to address elsewhere or at least after this is nailed down)? --- squirrelmail/functions/imap_messages.php 2008-01-15 16:34:31.000000000 +0100 +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 11:09:20.000000000 +0100 @@ -560,6 +560,9 @@ } $bUidFetch = ! in_array('UID', $aFetchItems, true); + + $temp = array(&$aHeaderFields, &$aFetchItems); + do_hook('small_header_list', $temp); /* Get the small headers for each message in $msg_list */ if ($msg_list !== NULL) { @@ -700,6 +700,8 @@ $aMsg['date'] = trim(str_replace(' ', ' ', $value)); break; case 'x-priority': $aMsg['x-priority'] = ($value) ? (int) $value{0} : 3; break; + case 'x-p772-primary-precedence': + $aMsg['x-p772-primary-precedence'] = $value; case 'priority': case 'importance': // duplicate code with Rfc822Header.cls:parsePriority() --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 16:34:17.000000000 +0100 +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php 2008-01-15 16:09:56.000000000 +0100 @@ -312,6 +312,8 @@ case 'x-mailer': $this->xmailer = $value; break; + case 'x-p772-primary-precedence': + $this->more_headers[$field] = $value; case 'x-priority': case 'importance': case 'priority': @@ -353,6 +355,9 @@ $this->x_spam_status = $this->parseSpamStatus($value); break; default: + if (substr($field, 0, 2) == 'x-') { + $this->more_headers[$field] = $value; + } break; } } |
From: Thierry G. <mel...@ya...> - 2008-02-07 08:18:46
|
--- Paul Lesniewski <pa...@sq...> wrote: > So do you agree with the following? Thierry, how about you? Does > this get everything (except the other next/previous links changes you > sent on this thread, which I'd like to address elsewhere or at least > after this is nailed down)? > > > --- squirrelmail/functions/imap_messages.php 2008-01-15 > 16:34:31.000000000 +0100 > +++ squirrelmail-melinda/functions/imap_messages.php 2008-01-18 > 11:09:20.000000000 +0100 > @@ -560,6 +560,9 @@ > } > > $bUidFetch = ! in_array('UID', $aFetchItems, true); > + > + $temp = array(&$aHeaderFields, &$aFetchItems); > + do_hook('small_header_list', $temp); > > /* Get the small headers for each message in $msg_list */ > if ($msg_list !== NULL) { So you move the hook into imap_messages.php ?... Well, it might result in more overhead (the hook being called each time the small header list function is called while not necessarily needed for the plugin itself). But if it is needed by other plugins then it's not a big issue as far as I am concerned... > @@ -700,6 +700,8 @@ > $aMsg['date'] = trim(str_replace(' > ', ' ', $value)); > break; > case 'x-priority': > $aMsg['x-priority'] = ($value) ? > (int) $value{0} : 3; break; > + case 'x-p772-primary-precedence': > + > $aMsg['x-p772-primary-precedence'] = $value; > case 'priority': > case 'importance': > // duplicate code with > Rfc822Header.cls:parsePriority() > --- squirrelmail/class/mime/Rfc822Header.class.php 2008-01-15 > 16:34:17.000000000 +0100 > +++ squirrelmail-melinda/class/mime/Rfc822Header.class.php > 2008-01-15 16:09:56.000000000 +0100 > > @@ -312,6 +312,8 @@ > case 'x-mailer': > $this->xmailer = $value; > break; > + case 'x-p772-primary-precedence': > + $this->more_headers[$field] = $value; > case 'x-priority': > case 'importance': > case 'priority': > @@ -353,6 +355,9 @@ > $this->x_spam_status = $this->parseSpamStatus($value); > break; > default: > + if (substr($field, 0, 2) == 'x-') { > + $this->more_headers[$field] = $value; > + } > break; > } > } Well, I'd rather have my newest patch implemented for these parts: it offers much more flexibility, including for "standard" priority headers (you could easily add support for X-MSMail-Priority and the like, for example, and this without touching the code: just with a few more entries in an array...); and actually I'm now using this flexibility, as the X-P772-Primary-Precedence field might actually hold different values in the future, depending from where the formal message comes (there will be gateways with the current ACP 127 messaging system, and there "routine" is used instead of "low", for example (specs are still a moving target on Melinda...). Did you see my email about the X-SMTP v2 patch, dated February the 5th ? If the new patch is not implemented, then you can already remove the X-P772-Primary-Precedence stuff from the above patch: I will simply provide a custom patch for my plugin... Regards, Thierry. ____________________________________________________________________________________ Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ |