Created attachment 46930 [details] [review]
ephy-auto-reload.patch

Build system patch for the new extension

Created attachment 46931 [details]
ephy-auto-reload.tar.gz

The files for the new extension

Thanks for the patch!

ephy-auto-reload-extension.h and .c have indentation and alignment problems.

struct _EphyAutoReloadExtensionPrivate
{
        gpointer dummy;
};

Unused; remove it and the g_type_class_add_private call in the class_init.

typedef struct
{
        EphyTab *tab;
        guint timeout;
        gboolean valid;
} TimeoutData;

This seems overly complicated to me; why not just set the return value from
g_timeout_add as data on the tab, and use the tab itself as data to
g_timeout_add ? You use .valid to invalidate the timeout in detach_tab; but you
should remove the timeout itself (since otherwise you'll get a crash when the
extension is unloaded and therefore the callback function not in memory anymore
when it's executed). (What about a tab_detach that happens when the user moves a
tab to another window? That shouldn't cancel the reload timeout...)

static gboolean
auto_reload_timeout (TimeoutData *timeout)
{       
        if ( !timeout->valid)
        {
                /* Old Timeout discard it */
                g_free (timeout);
                return FALSE;
        }
        
        if ( !EPHY_IS_TAB (timeout->tab))
                return FALSE;

see above. Oh, and you should use g_timeout_add_full with a suitable
GDestroyNotify func (probably g_free), instead of freeing that data from the
callback itself!


        EphyEmbed *embed = ephy_tab_get_embed (timeout->tab);
        if (embed == NULL)
                return FALSE;

That shouldn't happen, just use g_return_val_if_fail therefore.

        if (gtk_toggle_action_get_active (action) && new_timeout != 0)
        {
                LOG("Activated action");

It doesn't look like new_timeout could be 0 here?

  g_object_set_data (G_OBJECT (tab), TIMEOUT_DATA_KEY, timeout);
  g_timeout_add (new_timeout, (GSourceFunc) auto_reload_timeout, timeout);

See above.

        EphyTab *tab = ephy_window_get_active_tab (window);
        WindowData *data = (WindowData *) g_object_get_data (G_OBJECT (window),
WINDOW_DATA_KEY);
        g_return_if_fail (data != NULL);
        
        TimeoutData *timeout = (TimeoutData *) g_object_get_data (G_OBJECT
(tab), TIMEOUT_DATA_KEY);

No variable declaration after statements; that breaks compilation on gcc 2.95.

        if (new_active && !active)
                gtk_toggle_action_set_active (data->action, TRUE);
        else if (!new_active && active)
                gtk_toggle_action_set_active (data->action, FALSE);     

Don't you need to block the action callback here? (Else it'll activate the
callback).

        data->action_group = gtk_action_group_new
("EphyAutoReloadExtensionActions");
        gtk_action_group_set_translation_domain (data->action_group,
GETTEXT_PACKAGE);
        gtk_action_group_add_toggle_actions (data->action_group, action_entries,
G_N_ELEMENTS (action_entries), window);
        gtk_ui_manager_insert_action_group (manager, data->action_group, -1);
                
Add g_object_unref (data->action_group) or you'll leak the action group.

Created attachment 46952 [details]
ephy-auto-reload-2.tar.gz

Updated :
correct hopefully the indentation issues.
use as you said the EphyTab in the timeout callback
New algorithm: First a reload of 60 sec, then if the page hasn't changed reload
after x+60 sec, x being the old timeout value, if the page has changed reload
in x/2 seconds. This algorithm is not activated because there is need to detect
wether the page has changed, so filling the function and removing the comment
will activate it.

Comments are welcome :)

You might have to use some Mozilla C++ code to find whether the page has
changed. This may give a great benefit, too: Mozilla can ask the server whether
the page has changed without actually reading the page if it hasn't.

Many dynamic websites fake these details, though -- they'll set an "expire time"
in the past and set the "last updated" time to a very recent value, even though
the actual page hasn't changed. That practice is even suggested in PHP's
documentation, http://php.net/header. So while checking headers may be a good
idea, it may inspire your extension to reload every minute on pages which never
change. It is the intention of the page authors that their pages avoid being
cached, but reloading once per minute is excessive.

That was the idea, but i don't know yet anything of C++ and mozilla so i can't
do it right now.

Concerning the change detection, it is tricky and i don't know if there is a
reliable way. Maybe relying on the mozilla code, and setting a higher default
value, like 5 minutes could be a solution.

Otherwise the thing has to ask the user how much time to wait but i don't like
that, things are better when they Just Work :)

*** Bug 326616 has been marked as a duplicate of this bug. ***

Created attachment 119607 [details] [review]
make reload interval configurable

This patch introduces a new gconf property "/apps/epiphany/extensions/auto-reload/reload_interval" for the formerly hard coded reload interval.

> Concerning the change detection, it is tricky and i don't know if there is a
> reliable way. Maybe relying on the mozilla code, and setting a higher default
> value, like 5 minutes could be a solution.
> 
> Otherwise the thing has to ask the user how much time to wait but i don't like
> that, things are better when they Just Work :)

Hello!

For me it is a bug that the reload interval is hard coded (currently 3 minutes), but I agree with Raphael that we shouldn't ask the user about it. I don't know how to add an appropriate ui-element without negatively affecting usability, but nevertheless I wanna be able to change the reload interval without recompiling.

I've attached a patch which fixes that. With this patch applied, users can change the reload interval by launching gconf-editor and editing the key "/apps/epiphany/extensions/auto-reload/reload_interval". By default the key has the value 180, that is 180 seconds.

The question is how and where to document this. I would propose to add a point 5.3 "Configure Auto Reload Tab" to the Epiphany Extensions Manual.

I would be glad about some feedback...


Klaus

Well, there is an auto-reload extension now. This one seems to be Mozilla based. I do not know why it was never integrated or used, or this bug closed?

Feel free to reopen.