Patches for Squirrelmail SVN devel

View: New views
3 Messages — Rating Filter:   Alert me  

Patches for Squirrelmail SVN devel

by Thierry Godefroy-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Greetings !

Here are a few more patches (and a couple of questions) for Squirrelmail v1.5.2
SVN:

- OptionsCSS patch: fixes a problem in the latest SVN snapshots, where the
templates/default/css/options.css had a comment inserted wrongly (use of # to
mark the comment instead of /* */): this causes a lot of CSS errors in Firefox
(the errors are of course not show unless you open the error console).

- AutoRefresh patch: uses Javascript (when enabled) to refresh automatically
the left or right frame (refresh after a message is read, etc...). It is not
perfect as the left frame often gets refreshed while not really needed, but
it's a start and it does a good job eliminating the need to constantly click
the refresh link in the left frame... It gives SQM a much better and more
modern feel, IHMO.

- MelindaHooks v3 patch: this is the third (and normally, it should be the
final) version of my hooks patch for Melinda (but which could benefit other
plugins too): the change since the last version is in the number of parameters
passed to the new 'fetch_more_headers' hook (the last proposal to move this
hook out of mailbox_display.php is therefore no more suitable for my needs):
the name of the mailbox is now also displayed, so that the plugin may act
differently if it is to display a Sent or Inbox folder, for example (I use it
so that the Sent folders also shows workflow data as icons prefixing the
messages subjects in the list: for this to happen, I need to fetch one more
header for the Sent folder messages, and as this header is very large (it's a
X-RDF header with base64 encoded XML in it), I don't want to fetch it for the
other folders).

Now, a couple of questions:

1.- Don't you think it would be more advisable -NOT- to issue a DOCTYPE header
for the pages generated by Squirrelmail ?

The rationale behind this question is as follow:

a.- In its current state, SQM fails to generate legit XHTML 1.0 code: you can
check easily, by opening the error console of Firefox, or by trying to validate
at W3C.org a page generated by SQM.

b.- Because of the errors, some browsers (Firefox is one of them) reject many
CSS commands and the final look of the generated page is therefore different
from what it was supposed to be...

c.- No browser ever needed this info to properly generate a page, and in fact
all the browsers that I know of (and on which I ran tests) do a -MUCH- better
job when you don't restrict them to a particular DOCTYPE.

d.- While it's a Good Thing(tm) to try and stick strictly to a standard while
coding, restricting the rendering to that standard (via the DOCTYPE) narrows
the scope of potentially compatible clients (browsers).

Because of the above reasons, I disabled the echoing of the DOCTYPE (emitted by
templates/default/protocol_header.tpl) in my own installs of SQM, and so far,
Firefox, IE and others seem much happier this way...

2.- Any news about the various patches I submitted and that have not yet been
dealt with (either rejected or accepted) ?

I know that time is sparse and that everyone must be really busy with other
stuff than SQM, but some of the patches have been submitted a few weeks ago
already. As I am going to deploy SQM with my Melinda plugin, I need to write a
proper admin doc and to tell which patch might become part of SQM 1.5.2 or
not...

Below is the list of the pending submissions:

- XSMTP support: http://article.gmane.org/gmane.mail.squirrelmail.devel/9266

- Default settings for DB-based prefs:
http://article.gmane.org/gmane.mail.squirrelmail.devel/9257

- Address book bugs:
http://article.gmane.org/gmane.mail.squirrelmail.devel/9250 and
http://article.gmane.org/gmane.mail.squirrelmail.devel/9243 (this last one did
receive a partial answer, but the bug is still there...).

- RFC2047 encoded addresses bug:
http://article.gmane.org/gmane.mail.squirrelmail.devel/9237

Best regards,

Thierry Godefroy.



      ____________________________________________________________________________________
Never miss a thing.  Make Yahoo your home page.
http://www.yahoo.com/r/hs
diff -urN squirrelmail/templates/default/css/options.css squirrelmail-patched/templates/default/css/options.css
--- squirrelmail/templates/default/css/options.css 2008-03-05 01:43:18.000000000 +0100
+++ squirrelmail-patched/templates/default/css/options.css 2008-03-07 10:53:57.000000000 +0100
@@ -59,8 +59,8 @@
     padding-bottom: 2px;
     padding-left: 4px;
     padding-right: 4px;
-# FIXME: this was forcing horizontal scrolling on long coment texts and maybe other stuff; I'm not sure I see why we want this(?).  if someone restores it, PLEASE have a solution in mind for when an option widget has a 'comment' value that is quite long
-#    white-space: nowrap;
+/* FIXME: this was forcing horizontal scrolling on long comment texts and maybe other stuff; I'm not sure I see why we want this(?).  if someone restores it, PLEASE have a solution in mind for when an option widget has a 'comment' value that is quite long
+    white-space: nowrap; */
 }
 
 #optionDisplay  td.optionName   {

diff -urN squirrelmail/src/empty_trash.php squirrelmail-patched/src/empty_trash.php
--- squirrelmail/src/empty_trash.php 2007-07-14 19:30:44.000000000 +0200
+++ squirrelmail-patched/src/empty_trash.php 2008-03-07 14:20:10.000000000 +0100
@@ -72,5 +72,9 @@
 session_write_close();
 
 $location = get_location();
-header ("Location: $location/left_main.php");
+// We pass the name of the trash so that a check can be made and a decision can
+// be taken whether the right frame should be reloaded or not (it should if it
+// displays the trash contents). This can't be done here, because we can't emit
+// the necessary Javascript code while we are redirecting the page...
+header ("Location: $location/left_main.php?trash=$mailbox");
 
diff -urN squirrelmail/src/left_main.php squirrelmail-patched/src/left_main.php
--- squirrelmail/src/left_main.php 2007-07-14 19:30:44.000000000 +0200
+++ squirrelmail-patched/src/left_main.php 2008-03-07 14:20:42.000000000 +0100
@@ -209,3 +209,15 @@
 
 sqimap_logout($imapConnection);
 $oTemplate->display('footer.tpl');
+
+sqgetGlobalVar('trash', $trash, SQ_GET);
+if (isset($trash) && checkForJavascript()) {
+    // When Javascript is enabled, reload the right frame if it displays the
+    // trash contents and we just emptied the said trash (avoids being left
+    // with the deprecated contents of the now emptied trash folder, or a
+    // deleted message...).
+    echo '<script type="text/javascript">';
+    echo "var loc = '' + parent.right.location; if (loc.indexOf('mailbox=";
+    echo "$trash') > 0) parent.right.location = 'right_main.php?mailbox=$trash';";
+    echo "</script>\n";
+}
diff -urN squirrelmail/src/read_body.php squirrelmail-patched/src/read_body.php
--- squirrelmail/src/read_body.php 2008-03-06 04:50:56.000000000 +0100
+++ squirrelmail-patched/src/read_body.php 2008-03-07 14:21:30.000000000 +0100
@@ -916,6 +916,7 @@
 /**
  * update message seen status and put in cache
  */
+$was_unseen = !$message->is_seen;
 $message->is_seen = true;
 $aMailbox['MSG_HEADERS'][$passed_id]['MESSAGE_OBJECT'] = $message;
 
@@ -1017,3 +1018,10 @@
 do_hook('read_body_bottom', $null);
 sqimap_logout($imapConnection);
 $oTemplate->display('footer.tpl');
+
+if ($was_unseen && checkForJavascript()) {
+    // Reload the left frame if Javascript is enabled so to update the unread
+    // messages count.
+    echo '<script type="text/javascript">';
+    echo "parent.left.location = 'left_main.php';</script>\n";
+}
diff -urN squirrelmail/src/right_main.php squirrelmail-patched/src/right_main.php
--- squirrelmail/src/right_main.php 2007-09-25 06:49:39.000000000 +0200
+++ squirrelmail-patched/src/right_main.php 2008-03-07 14:21:06.000000000 +0100
@@ -364,6 +364,12 @@
 sqimap_logout ($imapConnection);
 $oTemplate->display('footer.tpl');
 
+if (checkForJavascript()) {
+    // Reload the left frame if Javascript is enabled (to update the unread
+    // messages count, for example...).
+    echo '<script type="text/javascript">';
+    echo "parent.left.location = 'left_main.php';</script>\n";
+}
 
 /* add the mailbox to the cache */
 $mailbox_cache[$account.'_'.$aMailbox['NAME']] = $aMailbox;

diff -urN squirrelmail/functions/mailbox_display.php squirrelmail-patched/functions/mailbox_display.php
--- squirrelmail/functions/mailbox_display.php 2008-02-10 16:45:01.000000000 +0100
+++ squirrelmail-patched/functions/mailbox_display.php 2008-03-07 09:22:48.000000000 +0100
@@ -315,6 +315,10 @@
       }
     }
 
+    // For X-SMTP headers we could need to fetch:
+    $temp = array(&$aHeaderFields, $aMailbox['NAME']);
+    do_hook('fetch_more_headers', $temp);
+
     /**
      * A uidset with sorted uid's is available. We can use the cache
      */
@@ -596,6 +600,7 @@
                         $title = ($sTmp != $value) ? preg_replace('/\s{2,}/', ' ', $value) : '';
                         $value = $sTmp;
                     }
+                    $value = (trim($value)) ? $value : _("(no subject)");
                     /* generate the link to the message */
                     if ($aQuery) {
                         // TODO, $sTargetModule should be a query parameter so that we can use a single entrypoint
@@ -606,10 +611,12 @@
                         // $onclick, $link_extra, $title, and so forth)
                         // plugins are responsible for sharing nicely (such as for
                         // setting the target, etc)
-                        $temp = array(&$iPageOffset, &$sSearch, &$aSearch, $aMsg);
+                        // We also pass the message subject in $value so that plugins
+                        // may change it (prefix it with a tag, for example) when they
+                        // need it.
+                        $temp = array(&$iPageOffset, &$sSearch, &$aSearch, $aMsg, &$value);
                         do_hook('subject_link', $temp);
                     }
-                    $value = (trim($value)) ? $value : _("(no subject)");
                     /* add thread indentation */
                     $aColumns[$k]['indent']  = $iIndent;
                     break;
diff -urN squirrelmail/src/image.php squirrelmail-patched/src/image.php
--- squirrelmail/src/image.php 2007-07-14 19:30:44.000000000 +0200
+++ squirrelmail-patched/src/image.php 2008-03-07 09:22:48.000000000 +0100
@@ -39,6 +39,11 @@
 
 $msg_url = 'read_body.php?' . $QUERY_STRING;
 $msg_url = set_url_var($msg_url, 'ent_id', 0);
+// With this hook, plugins can adjust the link if they recognize the
+// parent message as one they should take care about in place of SQM
+// default routines.
+$temp = array($mailbox, $passed_id, &$msg_url);
+do_hook('next_or_previous_link', $temp);
 echo '<a href="'.$msg_url.'">'. _("View message") . '</a>';
 
 
diff -urN squirrelmail/src/read_body.php squirrelmail-patched/src/read_body.php
--- squirrelmail/src/read_body.php 2008-03-06 04:50:56.000000000 +0100
+++ squirrelmail-patched/src/read_body.php 2008-03-07 09:22:48.000000000 +0100
@@ -515,6 +515,11 @@
         }
 
         $view_msg_href = $url;
+        // With this hook, plugins can adjust the link if they recognize the
+        // parent message as one they should take care about in place of SQM
+        // default routines.
+        $temp = array(&$aMailbox, $passed_id, &$view_msg_href);
+        do_hook('next_or_previous_link', $temp);
 
     // Prev/Next links for regular messages
     } else if ( true ) { //!(isset($where) && isset($what)) ) {
@@ -522,14 +527,26 @@
         $next = findNextMessage($aMailbox['UIDSET'][$what],$passed_id);
 
         if ($prev >= 0) {
-            $prev_href = $base_uri . 'src/read_body.php?passed_id='.$prev.
+            $prev_uri = $base_uri . 'src/read_body.php?passed_id='.$prev;
+            // With this hook, plugins can adjust the link if they recognize
+            // the previous message as one they should take care about in place
+            // of SQM default routines.
+            $temp = array(&$aMailbox, $prev, &$prev_uri);
+            do_hook('next_or_previous_link', $temp);
+            $prev_href = $prev_uri .
                    '&mailbox='.$urlMailbox.'&sort='.$sort.
                    "&where=$where&what=$what" .
                    '&startMessage='.$startMessage.'&show_more=0';
         }
 
         if ($next >= 0) {
-            $next_href = $base_uri . 'src/read_body.php?passed_id='.$next.
+            $next_uri = $base_uri . 'src/read_body.php?passed_id='.$next;
+            // With this hook, plugins can adjust the link if they recognize
+            // the next message as one they should take care about in place of
+            // SQM default routines.
+            $temp = array(&$aMailbox, $next, &$next_uri);
+            do_hook('next_or_previous_link', $temp);
+            $next_href = $next_uri .
                    '&mailbox='.$urlMailbox.'&sort='.$sort.
                    "&where=$where&what=$what" .
                    '&startMessage='.$startMessage.'&show_more=0';
@@ -540,7 +557,7 @@
         if ( $delete_prev_next_display == 1 &&
                in_array('\\deleted', $aMailbox['PERMANENTFLAGS'],true) ) {
             if ($prev >= 0) {
-                $del_prev_href = $base_uri . 'src/read_body.php?passed_id='.$prev.
+                $del_prev_href = $prev_uri .
                        '&mailbox='.$urlMailbox.'&sort='.$sort.
                        '&startMessage='.$startMessage.'&show_more=0'.
                        "&where=$where&what=$what" .
@@ -548,7 +565,7 @@
             }
 
             if ($next >= 0) {
-                $del_next_href = $base_uri . 'src/read_body.php?passed_id='.$next.
+                $del_next_href = $next_uri .
                        '&mailbox='.$urlMailbox.'&sort='.$sort.
                        '&startMessage='.$startMessage.'&show_more=0'.
                        "&where=$where&what=$what" .
diff -urN squirrelmail/src/vcard.php squirrelmail-patched/src/vcard.php
--- squirrelmail/src/vcard.php 2007-07-14 19:30:44.000000000 +0200
+++ squirrelmail-patched/src/vcard.php 2008-03-07 09:22:48.000000000 +0100
@@ -50,6 +50,11 @@
     '&startMessage='.urlencode($startMessage).
     '&passed_id='.urlencode($passed_id);
 $msg_url = set_url_var($msg_url, 'ent_id', 0);
+// With this hook, plugins can adjust the link if they recognize the
+// parent message as one they should take care about in place of SQM
+// default routines.
+$temp = array($mailbox, $passed_id, &$msg_url);
+do_hook('next_or_previous_link', $temp);
 
 $message = sqimap_get_message($imapConnection, $passed_id, $mailbox);
 
diff -urN squirrelmail/src/view_text.php squirrelmail-patched/src/view_text.php
--- squirrelmail/src/view_text.php 2007-07-14 19:30:44.000000000 +0200
+++ squirrelmail-patched/src/view_text.php 2008-03-07 09:22:48.000000000 +0100
@@ -50,6 +50,12 @@
 
 $msg_url   = 'read_body.php?' . $QUERY_STRING;
 $msg_url   = set_url_var($msg_url, 'ent_id', 0);
+// With this hook, plugins can adjust the link if they recognize the
+// parent message as one they should take care about in place of SQM
+// default routines.
+$temp = array($mailbox, $passed_id, &$msg_url);
+do_hook('next_or_previous_link', $temp);
+
 $dwnld_url = '../src/download.php?' . $QUERY_STRING . '&absolute_dl=true';
 $unsafe_url = 'view_text.php?' . $QUERY_STRING;
 $unsafe_url = set_url_var($unsafe_url, 'view_unsafe_images', 1);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Re: Patches for Squirrelmail SVN devel

by Thijs Kinkhorst :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Thierry,

On Friday 7 March 2008 15:12, Thierry Godefroy wrote:
> - OptionsCSS patch: fixes a problem in the latest SVN snapshots, where the
> templates/default/css/options.css had a comment inserted wrongly (use of #
> to mark the comment instead of /* */): this causes a lot of CSS errors in
> Firefox (the errors are of course not show unless you open the error
> console).

This has been merged in the meantime.

> - AutoRefresh patch: uses Javascript (when enabled) to refresh
> automatically the left or right frame (refresh after a message is read,
> etc...). It is not perfect as the left frame often gets refreshed while not
> really needed, but it's a start and it does a good job eliminating the need
> to constantly click the refresh link in the left frame... It gives SQM a
> much better and more modern feel, IHMO.

This looks like an ok thing to do to me, but refreshing the left frame
unneededly is a real issue if you ask me. It's a real performance/scalability
gain of SquirrelMail to not have to execute that code unneededly currently. I
would be willing to implement such a JavaScript refresh if it was reasonably
close to only refresh it "when needed".

Btw the current patch has an XSS hole in $trash (just for anyone planning to
commit it in the future, beware).

> - MelindaHooks v3 patch: this is the third (and normally, it should be the
> final) version of my hooks patch for Melinda (but which could benefit other
> plugins too): the change since the last version is in the number of
> parameters passed to the new 'fetch_more_headers' hook (the last proposal
> to move this hook out of mailbox_display.php is therefore no more suitable
> for my needs): the name of the mailbox is now also displayed, so that the
> plugin may act differently if it is to display a Sent or Inbox folder, for
> example (I use it so that the Sent folders also shows workflow data as
> icons prefixing the messages subjects in the list: for this to happen, I
> need to fetch one more header for the Sent folder messages, and as this
> header is very large (it's a X-RDF header with base64 encoded XML in it), I
> don't want to fetch it for the other folders).

Ok, as said earlier, I'm staying away from hook-based stuff for now..

> Now, a couple of questions:
>
> 1.- Don't you think it would be more advisable -NOT- to issue a DOCTYPE
> header for the pages generated by Squirrelmail ?
>
> The rationale behind this question is as follow:
>
> a.- In its current state, SQM fails to generate legit XHTML 1.0 code: you
> can check easily, by opening the error console of Firefox, or by trying to
> validate at W3C.org a page generated by SQM.
>
> b.- Because of the errors, some browsers (Firefox is one of them) reject
> many CSS commands and the final look of the generated page is therefore
> different from what it was supposed to be...
>
> c.- No browser ever needed this info to properly generate a page, and in
> fact all the browsers that I know of (and on which I ran tests) do a -MUCH-
> better job when you don't restrict them to a particular DOCTYPE.
>
> d.- While it's a Good Thing(tm) to try and stick strictly to a standard
> while coding, restricting the rendering to that standard (via the DOCTYPE)
> narrows the scope of potentially compatible clients (browsers).
>
> Because of the above reasons, I disabled the echoing of the DOCTYPE
> (emitted by templates/default/protocol_header.tpl) in my own installs of
> SQM, and so far, Firefox, IE and others seem much happier this way...

This is of course a tradeoff, and for stable, our aim is to be fully compliant
so any deviation is a bug. For devel, since it's in a transitional period
(way to long.... but that's reality) there's currently a phase where both the
HTML is not valid and there's still a doctype. But that's expected to
converge back to validity at some point.

> 2.- Any news about the various patches I submitted and that have not yet
> been dealt with (either rejected or accepted) ?
>
> I know that time is sparse and that everyone must be really busy with other
> stuff than SQM, but some of the patches have been submitted a few weeks ago
> already. As I am going to deploy SQM with my Melinda plugin, I need to
> write a proper admin doc and to tell which patch might become part of SQM
> 1.5.2 or not...

As you can see I'm doing my best on working through your submissions. As said
we really appreciate your work.


cheers,

Thijs

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Re: Patches for Squirrelmail SVN devel

by Thierry Godefroy-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

--- Thijs Kinkhorst <kink@...> wrote:

> Hi Thierry,
>
> On Friday 7 March 2008 15:12, Thierry Godefroy wrote:
>
> .../...
>
> > - AutoRefresh patch: uses Javascript (when enabled) to refresh
> > automatically the left or right frame (refresh after a message is read,
> > etc...). It is not perfect as the left frame often gets refreshed while not
> > really needed, but it's a start and it does a good job eliminating the need
> > to constantly click the refresh link in the left frame... It gives SQM a
> > much better and more modern feel, IHMO.
>
> This looks like an ok thing to do to me, but refreshing the left frame
> unneededly is a real issue if you ask me. It's a real performance/scalability
> gain of SquirrelMail to not have to execute that code unneededly currently. I
> would be willing to implement such a JavaScript refresh if it was reasonably
> close to only refresh it "when needed".
>
> Btw the current patch has an XSS hole in $trash (just for anyone planning to
> commit it in the future, beware).

Well, having the users constantly clicking on the "Get new mail" link in the
left frame just to be sure they got it up to date is not really a gain at all
either... ;-P
But, yes, the patch would need some serious work on it (like I wrote: it's just
a start.... kind of a draft patch :-)
The problem is that I'm currently busy writing the proper development
documentation to handover my work to another developer, and I'm not sure I will
have enough time to work more on this patch... I will try, but don't hold your
breath on it.

> .../...
>
> > 2.- Any news about the various patches I submitted and that have not yet
> > been dealt with (either rejected or accepted) ?
> >
> > I know that time is sparse and that everyone must be really busy with other
> > stuff than SQM, but some of the patches have been submitted a few weeks ago
> > already. As I am going to deploy SQM with my Melinda plugin, I need to
> > write a proper admin doc and to tell which patch might become part of SQM
> > 1.5.2 or not...
>
> As you can see I'm doing my best on working through your submissions. As said
> we really appreciate your work.

Thank you for all your replies ! :-)

Best regards,

Thierry.


      ____________________________________________________________________________________
Be a better friend, newshound, and
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel
LightInTheBox - Buy quality products at wholesale price