706414 - Clear History… menu item is disabled when history is disabled, preventing clearing Recently Closed Pages

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[Oufcom]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20111113195207

Steps to reproduce:

since the last version of camino ( Version 2.1 (1.9.2.24 20111113195207) )
empty history is not available any more



Actual results:

it's not anymore possible to empty history from interface (history, empty history) 
if expiration is set at 0 day(s)


Expected results:

it should stay available after first page seen :

that feature is usefull for anyone who want to clear history WITHOUT restarting browser (for what i know, it still works if we go in preference > history > clear history )

Comment 1

[philippe (part-time)]

Do you mean the menu item is not available from the History menu ?
The menu item is labeled 'clear history…'. It is at the bottom.

Comment 2

[Oufcom]

exactly,
this is "clear history" in "history menu" 
(i'm sorry, i'm using french version of browser, so i may have made a mistake on label)

Comment 3

[Chris Lawson (gone)]

Are you using the French (multilingual) version or the English version of Camino?

Are you saying the menu item is simply not present in the browser at all? (Its location is accurately described in comment 1.)

If that's the case, can you try downloading Camino again, and seeing if the menu item appears?

Comment 4

[Oufcom]

- French (multilingual) of camino
- he is present but unavailable (in gray), even after many page visited

i downloaded again multilingual version, installed (currently using it), i have exactly the same problem

Comment 5

[Oufcom]

ps : i'm 100% sure this problem didn't exist on the previous version of camino (i'm using this features each day)

by the way, thank you for taking time to check this :)

Sil

Comment 6

[philippe (part-time)]

Now I understand :-)
You have the History preferences set to 0 days. In that case the 'Clear History…' menu item is indeed grayed out. That is correct I think, as no history is ever recorded. You can open the History manager (Cmd-Y) to verify. It will remain empty after visiting (multiple) pages.

There is one inconsistency though: past pages from the current session can still be accessed through the location bar autocomplete.

Comment 7

[Oufcom]

in fact, some type of history is recorded in "Pages recently closed" (i hope label is not to far from english version)
in any case, i know this is cleared when you leave the browser, 
but "clear history" was a usefull solution to make this list empty without exiting browser

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Almost certainly caused by bug 589518 :-(

Stuart, what should we do here?  Back out bug 589518 entirely (thereby restoring the original bug of an enabled menu item when there's no history or RCPs), or tweak the check to also make sure there's also nothing in "Recently Closed Pages"?

Oufcom: thanks for reporting this.  In the future, if you file the bug with the version of Camino that exhibits the bug, we'd be able to tell right away you were using the French version, and so forth ;-)

(In reply to philippe from comment #6)
> There is one inconsistency though: past pages from the current session can
> still be accessed through the location bar autocomplete.

This probably needs a new bug, because the string says that won't happen, so we either need to change the string, or make sure the Trie is purged :/

Comment 9

[Stuart Morgan]

I think the easiest fix would probably be to just make it so that it doesn't ever disable if the max history is set to 0; that's less work that plumbing the recent items stuff, and should be the same except for the edge case's edge case of having it set to 0, clearing, and then immediately trying to clear again... and if someone in that case wants to clear compulsively, more power to them ;)

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to Stuart Morgan from comment #9)
> I think the easiest fix would probably be to just make it so that it doesn't
> ever disable if the max history is set to 0; that's less work that plumbing

So the only cases where we'd ever disable it would then be

1) a fresh profile before a page has loaded, and
2) immediately after history has been cleared, when a user keeps history

?

And so the existing check would become something like

return (HasHistoryItems() && kGeckoPrefHistoryLifetimeDays > 0);

?

> that's less work that plumbing the recent items stuff

That seemed pretty easy, except I couldn't find the magic incantation I needed to make MC call my new hasRecentlyClosedPages method in HistoryMenu :P

Comment 11

[Oufcom]

Smokey Ardisson : sorry i totally forgot i'm using another user agent, my bad >< 
(not always possible to use some website with camino agent when they only support "common" browsers like gmail and so one) :)

thank you all for your support ;)

regards,
Sil

Comment 12

[Stuart Morgan]

(In reply to Smokey Ardisson (back-ish; no bugmail - do not email) from comment #10)
> And so the existing check would become something like
> 
> return (HasHistoryItems() && kGeckoPrefHistoryLifetimeDays > 0);

Exactly (modulo the constant not actually being the pref value, of course ;) )

But you are right, it would probably not be hard to do with HistoryMenu if we wanted to go that route.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Quick solution

This implements Stuart's suggested solution, which is quick and simple and has no l10n impact.  (It does, however, have another edge case, for the non-0-history people: clear history, close a page, try to clear history again to clear the RCP and have a disabled menu item.)

We may want to take this and delay a better fix until later.

The better fix, what I suggested, is going to require a bit of plumbing, because I was trying to call an instance method as a class method, and we have no instance of the TopLevelHistoryMenu, since it's set up only in the nib, so we'd have to add an outlet to the nib.  It probably wouldn't be hard to do (and fix up everyone's nib ourselves); just not sure if it's worth it for 2.1.x.

Comment 14

[BuggyUK]

Since upgrading to Version 2.1 (1.9.2.24 20111113195207)
my history in lib, apps, etc has stopped updating, which I usually copy on a daily basis for backups.

Where is history in Version 2.1 (1.9.2.24 20111113195207) stored now ?
Has it become invisible, or have a different name



History is still visible and updating ok within camino Since upgrading to Version 2.1 (1.9.2.24 20111113195207)

Comment 15

[Chris Lawson (gone)]

(In reply to d.mus from comment #14)
> Since upgrading to Version 2.1 (1.9.2.24 20111113195207)
> my history in lib, apps, etc has stopped updating, which I usually copy on a

Do you have DNS prefetch disabled?

If so, you're suffering from bug 705610.

Comment 16

[philippe (part-time)]

(In reply to d.mus from comment #14)

> Where is history in Version 2.1 (1.9.2.24 20111113195207) stored now ?
> Has it become invisible, or have a different name

History is now stored in places.sqlite (inside your profile folder)

Comment 17

[BuggyUK]

Places.sqlite,   thanks

I notice that the file size is very much reduced compared to previous history, compression ?

Previous history was 50mb, Places.sqlite is only 9mb, and seems to contain all previous and new history.

Comment 18

[Hope Stuckman]

what does it mean if I am on the CC list I do not know much about computers?

Comment 19

[Stuart Morgan]

BuggyUK: if you want to have a general discussion about history storage, use the forums linked on the contact page. Hijacking someone's bug report to ask unrelated questions is kind of rude, and makes our job harder.

Comment 20

[BuggyUK]

(In reply to Stuart Morgan from comment #19)
> BuggyUK: if you want to have a general discussion about history storage, use
> the forums linked on the contact page. Hijacking someone's bug report to ask
> unrelated questions is kind of rude, and makes our job harder.

Pardon me for using search and being directed to this thread !!

Thank you for the two with the courtesy to reply in the positive.

Now get back on track !

Comment 21

[philippe (part-time)]

(In reply to Smokey Ardisson (back-ish; no bugmail - do not email) from comment #8)
 
> (In reply to philippe from comment #6)
> > There is one inconsistency though: past pages from the current session can
> > still be accessed through the location bar autocomplete.
> 
> This probably needs a new bug, because the string says that won't happen, so
> we either need to change the string, or make sure the Trie is purged :/

Actually, after testing this again with a build with the patch for bug 705610, I can't reproduce it anymore. With history set to '0', pages visited dring the session can't accessed through the location bar autocomplete (unless they're bookmarked of course).

Comment 23

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Better (preferred) solution

This is my preferred solution, provided I can make IB make the connection in all the the nibs (it proved to be somewhat of a challenge in English, but I think I have the magic gesture down now) :P

Most of the diff is the re-indenting of the @interface for MainController to match the new width of the "columns" :P  Since I was already churning there, I fixed the comment style for all of those items.

I assumed I needed to add @class TopLevelHistoryMenu there, too, because the other outlets and things that showed up in the @interface also had their classes listed.

The rest of the changes should be pretty straightforward, and I don't have any questions about what I did in those parts ;-)

I'm going to request review on both patches so we'll have flexibility, but I do prefer this one, assuming iLocalize hasn't garbled the nibs so that I can't open them (or that l10n will pull a nightly and update this one nib).

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: updated MainMenu.nib to accompany Better (preferred) solution

Comment 25

[Stuart Morgan]

Comment on attachment 581415 [details] [diff] [review]
Better (preferred) solution

sr=smorgan

Comment 26

[Stuart Morgan]

Comment on attachment 578813 [details] [diff] [review]
Quick solution

sr=smorgan

Comment 27

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: updated MainMenu.nib for fr, it, ja, nb, nl, and sv

Here are the updated MainMenu.nibs for Dutch, French, Italian, Japanese, Norwegian, and Swedish (attached here for convenient storage).  I can't open the Chinese MainMenu.nib, so when we hear from Tianhao again, we'll have to assess that situation.

Comment 28

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Better (preferred) solution landed as http://hg.mozilla.org/camino/rev/ac4af74b652f