1913787 - Replace session history in the Go Menu with places history

Status: ASSIGNED

(SeaMonkey :: Bookmarks & History, enhancement)

Description

[Matt A. Tobin [:nsITobin]]

With the places refresh a while back updating the UX and the simple fact that single session (current tab) history in a primary menu is overall less useful for tabbed browsing. Not to mention, the advent of JavaScript Web Applications rewriting session history which further reduces the usefulness of it.

To resolve (sidestep?) these issues this bug swaps session history population of the Go menu with global Places history items effectively making it a hybrid between the classic Go menu and the Firefox History menu.

Comment 1

[Matt A. Tobin [:nsITobin]]

Attachment: places-go-menu-v1.patch

Comment 2

[Matt A. Tobin [:nsITobin]]

Attachment: places-go-menu-v1.1

Fix typo (boomarks)

Comment 3

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9419742 [details] [diff] [review]
places-go-menu-v1.1

I am rarely if ever use the menu so ok with it. Only use the toolbar buttons. If I need history I usually use the library directly so someone else should comment if we want this.

Comment 4

[Matt A. Tobin [:nsITobin]]

NOTE: Removed code includes previously commented out SyncUI code that would need to be restored from elsewhere when Sync comes back.

Comment 5

[Ian Neal]

Comment on attachment 9419742 [details] [diff] [review]
places-go-menu-v1.1

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

::: suite/browser/navigatorOverlay.xul
@@ +451,5 @@
>            label="&goMenu.label;"
> +          accesskey="&goMenu.accesskey;">
> +      <menupopup id="historyPopup"
> +                 placespopup="true"
> +                 context="placescontext"

shouldn't this be placesContext?
With that change all the menu items that are not part of places have "Open All in Tabs", that probably needs fixing.

@@ +455,5 @@
> +                 context="placescontext"
> +                 oncommand="BookmarksEventHandler.onCommand(event);"
> +                 onclick="BookmarksEventHandler.onClick(event);"
> +                 onpopupshowing="updateRecentMenuItems();
> +                                 if (!this.parentNode._placesView) { new HistoryMenu(event); }">

As this is the only place that calls updateRecentMenuItems can the onpopupshowing call be made simpler by passing suitable arguments to the function and moving the if statement into the function?

Comment 6

[Matt A. Tobin [:nsITobin]]

In my original modification for Borealis I was not able to get a good handle on the places context menu due to its incompleteness in its 2.49 form so I left it out and because side-by-side with the Go menu it didn't seem to offer much advantage since SeaMonkey's UI is more about managers than context menu items on other menu items.

While the places implementation is now more complete it still isn't 1:1 with Firefox yet so I don't know how complex the logic would be to fully enable the context menu. More research and work into aligning the navibrowser side of it to Firefox would likely need done.