Confirmed!

Still valid in 2.27.5 with webkit as backend

I was writing some small patches to fix this issue, the idea behind them is basically:

  - When the user confirms on "Clearing the history", the Back and Forward buttons in the toolbars for every window get disabled, as there shouldn't be anything to navigate through. 

  - When the EphyHistory gets cleared, every EphyEmbed handles the 'cleared' signal and tells their EphyWebView's to clear their WebKitWebBackForwardList's, so that makes the model coherent with the UI.

It's a set of 4 small patches I'm attaching right now one by one.

Created attachment 158083 [details] [review]
Added new function to clear the history from WebKitWebView

Just retrieves the WebKitWebBackForwardList and clear it.

Created attachment 158084 [details] [review]
2. Make sure WebKitWebHistory is cleared when cleared EphyHistory

Connect to the 'cleared' signal and call to
ephy_web_view_clear_history.

Created attachment 158085 [details] [review]
3. Split ephy_toolbar_set_navigation_actions in two functions

Provide ephy_toolbar_set_up_action and
ephy_toolbar_set_back_forward_actions instead to allow more
flexibility when needed to just disable the Back and Forward buttons,
for instance.

Created attachment 158086 [details] [review]
4. Disable Back and Forward buttons after clearing the history 

Do it for every opened window in the current session.

As per Xan comments (in-real-life comments, I mean), I tried a different approach for this bug, which basically replaces the 3rd and 4th patches this way:

Create two new subclasses of the EphyNavicationAction class, EphyNavigationHistoryAction and EphyNavigationUpAction, leaving the common logic in the super class (abstract from now on) and the very specific one of each case in every subclass.

Done this, listen from EphyNavigationHistoryAction for the "cleared" signal from EphyHistory to know when the Action must disable itself in the toolbar, therefore not needing to explicitly do, from an external object, something like it was done in the 4th path before.

Also, the 1st and 2nd patches were refactorized in a better way, and the ephy_web_view_clear_history() function is now a more correct one, taking care of not leaving the WebKitWebHistory completely empty (make sure the current item remains) and of updating the navigation flags as needed.

So, now attaching the new patches...

Created attachment 158470 [details] [review]
1. Make sure WebKitWebHistory is cleared when cleared EphyHistory

Added new function in EphyWebView to clear the history from WebKitWebView, and connect to the 'cleared' signal in EphyEmbed to call to such a function when needed.

Created attachment 158471 [details] [review]
2. Split EphyNavigationAction in one abstract class and two subclasses

To ease understanding and further modification of the two different usages for the EphyNavigationAction class (Back/Forward and Up buttons), all the code there was splitted so the common one is kept in the superclass, delegating through a template pattern some parts in the specific implementations: History (back/forward) and Up buttons.

Created attachment 158472 [details] [review]
3. Change sensitiveness for history buttons when clearing the history

Connect to the 'cleared' signal and change the sensitivity flags

Review of attachment 158470 [details] [review]:

Humble review: looks good!

Now that you and Xan are playing with this class, would you guys take a look at bug #611400?
I added a comment there.

(In reply to comment #13)
> Now that you and Xan are playing with this class, would you guys take a look at
> bug #611400?
> I added a comment there.

Sure! I'll take a look on them tomorrow morning.

Thanks for quickly reviewing my patches. Feel free to spot anything you find wrong/to be improved/whatever.

Review of attachment 158470 [details] [review]:

Looks fine except for a couple of details.

::: embed/ephy-web-view.c
@@ +2186,3 @@
+ *
+ * @view: the #EphyWebView to clear the history from
+ * ephy_web_view_clear_history:

* is in the wrong place.

@@ +2203,3 @@
+    /* Get rid of the extra ref now */
+    g_object_unref (current_item);
+

I think we could do without that comment ;)

@@ +2205,3 @@
+
+    /* Update navigation flags after clearing WebKit history up */
+    update_navigation_flags (view);

And this one.

Review of attachment 158471 [details] [review]:

Looks good minus the details commented. In the commit message, I think 'splitted' is written 'split'. Also don't say "template pattern" for the love of $DEITY :D

::: src/ephy-navigation-history-action.c
@@ +1,2 @@
+/*
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */

This does not seem to be what you are actually using in the file? It's not the new recommended style in any case. Also, all the braces in the file aren't following the new style either.

@@ +67,3 @@
+	       ephy_navigation_history_action,	\
+	       EPHY_TYPE_NAVIGATION_ACTION)
+

No need to put this in multiple lines.

@@ +336,3 @@
+      nav->priv->direction = g_value_get_int (value);
+      break;
+    }

Missing default with the usual warning.

@@ +352,3 @@
+      g_value_set_int (value, nav->priv->direction);
+      break;
+    }

Same thing.

@@ +361,3 @@
+  GtkActionClass *action_class = GTK_ACTION_CLASS (class);
+  EphyNavigationActionClass *nav_action_class = EPHY_NAVIGATION_ACTION_CLASS (class);
+

While you are at it you could probably use 'klass' here.

::: src/ephy-navigation-history-action.h
@@ +59,3 @@
+{
+	EphyNavigationActionClass parent_class;
+};

Too much indentation?

::: src/ephy-navigation-up-action.c
@@ +1,2 @@
+/*
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */

Same comment than in the other file.

@@ +47,3 @@
+	       ephy_navigation_up_action,	\
+	       EPHY_TYPE_NAVIGATION_ACTION)
+

And same here.

@@ +196,3 @@
+  G_OBJECT_CLASS (ephy_navigation_up_action_parent_class)->finalize (object);
+}
+

No need to define a finalize method if you are not going to do anything.

::: src/ephy-navigation-up-action.h
@@ +50,3 @@
+	EphyNavigationActionClass parent_class;
+};
+

Again indentation.

Review of attachment 158472 [details] [review]:

Eh, OK, so in the previous patch you were connecting the signal but the callback wasn't defined? That's evil. Just fold this into the previous patch.

Created attachment 159147 [details] [review]
1. Make sure WebKitWebHistory is cleared when cleared EphyHistory

(In reply to comment #15)
> Review of attachment 158470 [details] [review]:
> 
> Looks fine except for a couple of details.
> 
> ::: embed/ephy-web-view.c
> @@ +2186,3 @@
> + *
> + * @view: the #EphyWebView to clear the history from
> + * ephy_web_view_clear_history:
> 
> * is in the wrong place.

What do you mean? I placed this function right after ephy_web_view_copy_back_history because it made sense to me so I'm not sure where you would put it.

If your talking about the title of the function being in the second line of the comment I don't know what happened there... in the patch there are in the right order :-/

> @@ +2203,3 @@
> +    /* Get rid of the extra ref now */
> +    g_object_unref (current_item);
> +
> 
> I think we could do without that comment ;)

Done

> @@ +2205,3 @@
> +
> +    /* Update navigation flags after clearing WebKit history up */
> +    update_navigation_flags (view);
> 
> And this one.

Done

Attaching new patch

Created attachment 159148 [details] [review]
2. Split EphyNavigationAction in one abstract class and two subclasses

(In reply to comment #16)
> Review of attachment 158471 [details] [review]:
> 
> Looks good minus the details commented. In the commit message, I think
> 'splitted' is written 'split'. Also don't say "template pattern" for the love
> of $DEITY :D
> [...]

Attaching patch addressing all these issues.

Created attachment 159149 [details] [review]
2. Split EphyNavigationAction in one abstract class and two subclasses

(In reply to comment #17)
> Review of attachment 158472 [details] [review]:
> 
> Eh, OK, so in the previous patch you were connecting the signal but the
> callback wasn't defined? That's evil. Just fold this into the previous patch.

WTF?? Damm it! It seems that I screwed up when preparing patches with git rebase -i and that I put some code from the third patch in the second one, along with the splitting in two subclasses.

Now attaching the new patches...

Created attachment 159150 [details] [review]
3. Change sensitiveness for history buttons when clearing the history

New version of the third patch

sorry for the spam :/

(In reply to comment #18)
> Created an attachment (id=159147) [details] [review]
> 1. Make sure WebKitWebHistory is cleared when cleared EphyHistory
> 
> (In reply to comment #15)
> > Review of attachment 158470 [details] [review] [details]:
> > 
> > Looks fine except for a couple of details.
> > 
> > ::: embed/ephy-web-view.c
> > @@ +2186,3 @@
> > + *
> > + * @view: the #EphyWebView to clear the history from
> > + * ephy_web_view_clear_history:
> > 
> > * is in the wrong place.
> 
> What do you mean? I placed this function right after
> ephy_web_view_copy_back_history because it made sense to me so I'm not sure
> where you would put it.
> 

Not sure what happened with the review stuff, but I meant to point out that:

+  WebKitWebBackForwardList* history_list;

has the '*' in the wrong position.

Comment on attachment 159147 [details] [review]
1. Make sure WebKitWebHistory is cleared when cleared EphyHistory

Looks good minus the small style thing, I'll fix before committing.

Comment on attachment 159150 [details] [review]
3. Change sensitiveness for history buttons when clearing the history

Looks good.

Comment on attachment 159149 [details] [review]
2. Split EphyNavigationAction in one abstract class and two subclasses

Looks good!

I pushed all three patches to master, fantastic job!

(In reply to comment #26)
> I pushed all three patches to master, fantastic job!

Thanks, and sorry for the style thing.