928640 - Opening 'recent closed tabs' item with middle-click is broken

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/e25e62d174ed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131019030204

Steps To Reproduce:
1. Close several tabs
2. History -> Recently close tabs -> mid-click on any 'recent closed tab'

Actual Results:
Nothing happens

Expected Results:
Should open in a 'new tab'


Regression window:
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=50abdc9ee405&tochange=1dd4cd4890fc

Comment 1

[:Gijs (he/him)]

http://hg.mozilla.org/integration/fx-team/rev/082357277604#l2.63


    2.63 +        element.addEventListener("click", this._undoCloseMiddleClick, false);

Except there is no this._undoCloseMiddleClick.

This will also be broken on UX, I'd imagine.

Comment 2

[tabmix.onemen]

Restore All Tabs is also broken 

> Error: undoCloseTab is not defined
> Source File: resource://app/modules/sessionstore/RecentlyClosedTabsAndWindowsMenuUtils.jsm
> Line: 70

getTabsFragment use addEventListener for "click" and "command" - undoCloseTab  and this._undoCloseMiddleClick are both not defined.

getWindowsFragment use "oncommand" and the reference for undoCloseWindow is in string that only use form browser window

Comment 3

[:Gijs (he/him)]

Attachment: middle click open recent tabs and restore all tabs broken,

This fixes things for me, and seems to make sense...

Comment 4

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

The patch from bug 927917 needs to be merged to m-c, and then this patch can build on top of that one.

Comment 5

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

(In reply to Jared Wein [:jaws] from comment #4)
> The patch from bug 927917 needs to be merged to m-c, and then this patch can
> build on top of that one.

This was completely incorrect, as the patch in that bug only touched UX-specific code.

Comment 6

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

Comment on attachment 819500 [details] [diff] [review]
middle click open recent tabs and restore all tabs broken,

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

::: browser/components/sessionstore/src/RecentlyClosedTabsAndWindowsMenuUtils.jsm
@@ +133,5 @@
> +    * Used for a middle click.
> +    * @param aEvent
> +    *        The event when the user clicks the menu item
> +    */
> +  _undoCloseMiddleClick: function PHM__undoCloseMiddleClick(aEvent) {

PHM__ doesn't make any sense here. Since this is in a JSM, we can also remove the underscore prefix from the function name.

Comment 7

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] from comment #6)
> Comment on attachment 819500 [details] [diff] [review]
> middle click open recent tabs and restore all tabs broken,
> 
> Review of attachment 819500 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> browser/components/sessionstore/src/RecentlyClosedTabsAndWindowsMenuUtils.jsm
> @@ +133,5 @@
> > +    * Used for a middle click.
> > +    * @param aEvent
> > +    *        The event when the user clicks the menu item
> > +    */
> > +  _undoCloseMiddleClick: function PHM__undoCloseMiddleClick(aEvent) {
> 
> PHM__ doesn't make any sense here. Since this is in a JSM, we can also
> remove the underscore prefix from the function name.

I've removed the function name, like the other functions in that object.

https://hg.mozilla.org/integration/fx-team/rev/f77442b18813

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f77442b18813