870461 - Add 'Clear Recent History', 'Restore Previous Session', and tweak some of the text

Status: RESOLVED FIXED

(Firefox :: Menus, enhancement)

Description

[u428464]

I think the history subview could be done differently.

There should be first three general items :
-Show all history instead of "more" at the bottom.
-Clear recent History... -> bring the clear history dialog (or second subview).
-Restore previous session.

If possible "Recently closed" (tabs and windows will be merged) -> this too would need UI (second subview ?).

And then the history last items.

If general commands aren't present it would be a pretty big regression compared to the old menu and these items would be accessible only via the menu bar and shortcuts. 

I know that subview inside a subview looks a bit "Inception-esque" but for certain items it should be at least considered.

Comment 1

[u428464]

The tooltip of the widget should also be changed since "History repeats itself" is fun, but not very useful.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I left out the Recently Closed Tabs + Windows for now since it will probably require another subview or some other UI that we haven't figured out yet.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Note that the attached patch is dependent upon the patch in bug 874319.

Comment 4

[:Gijs (he/him)]

Comment on attachment 752022 [details] [diff] [review]
Patch

Review of attachment 752022 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/projects/ux/rev/03bae1732987

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 752022 [details] [diff] [review]
Patch

> <!ENTITY historyHomeCmd.label "Home">
> <!ENTITY showAllHistoryCmd2.label "Show All History">
> <!ENTITY showAllHistoryCmd.commandkey "H">
> 
> <!ENTITY appMenuCustomize.label "Customize">
> <!ENTITY appMenuBookmarks.label "Bookmarks">
> <!ENTITY appMenuHistory.label "History">
>-<!ENTITY appMenuHistoryMore.label "Moreâ¦">
>+<!ENTITY appMenuHistory.moreLabel "Show All History">
>+<!ENTITY appMenuHistory.clearRecentLabel "Clear Recent Historyâ¦">
>+<!ENTITY appMenuHistory.restoreSession "Restore Previous Session">

You missed the 'label' suffix for restoreSession. Also, where you do have the suffix, it's weird that you didn't separate it with a dot like it's usually done for label, accesskey, description, commandkey, tooltip etc.

Comment 7

[:Gijs (he/him)]

Attachment: Change l10n ids per dao's feedback

> You missed the 'label' suffix for restoreSession. Also, where you do have
> the suffix, it's weird that you didn't separate it with a dot like it's
> usually done for label, accesskey, description, commandkey, tooltip etc.

As this is not on m-c yet, figure we might as well change this still?

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 752642 [details] [diff] [review]
Change l10n ids per dao's feedback

appMenuHistory.more.label should probably be appMenuHistory.showAll.label?

Comment 9

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 752642 [details] [diff] [review]
> Change l10n ids per dao's feedback
> 
> appMenuHistory.more.label should probably be appMenuHistory.showAll.label?

Pushed to UX with that change: https://hg.mozilla.org/projects/ux/rev/1718434171c2

Comment 10

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/03bae1732987
https://hg.mozilla.org/mozilla-central/rev/1718434171c2