Tabbed browser doesn't tell session restore to disable undo close tab

RESOLVED FIXED

Status

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: neil, Assigned: neil)

Tracking

({regression})

Trunk
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

934 bytes, patch
iann_bugzilla
: review+
misak.bugzilla
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
The tabbed browser's removeTab method has an aDisableUndo argument but that only disables the tabbrowser's undo and not the session restore undo close tab.

This has become significant because of bug 530735 which allows the session restore undo close tab to operate on the tab.
(Assignee)

Comment 1

9 years ago
Created attachment 426231 [details] [diff] [review]
Proposed patch

The session store code already expects a UIEvent with a detail anyway.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #426231 - Flags: review?(misak)
Attachment #426231 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 2

9 years ago
Misak, I don't suppose that this is something that you can write a test for?

Comment 3

9 years ago
Comment on attachment 426231 [details] [diff] [review]
Proposed patch

I think i should add code to handle aDisableUndo in sessionrestore.
Attachment #426231 - Flags: review?(misak) → review+
(Assignee)

Comment 4

9 years ago
It's already there in line 419 of nsSessionStore.js:
if (!aEvent.detail)
  this.onTabClose(win, aEvent.originalTarget);
Its purpose was originally to deal with switching tabs between browsers, where you also want to disable undo.

Comment 5

9 years ago
Ah, I'm starring to the code two days to find how it should work. Thanks for explanation :)
(Assignee)

Comment 6

9 years ago
(In reply to comment #5)
> Ah, I'm starring to the code two days to find how it should work. Thanks for
> explanation :)
No, starring is what you do to non-green Tinderboxes ;-) (Mind you, many developers seem to be staring at the non-green Tinderboxes, not that it helps!)

Updated

9 years ago
Attachment #426231 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 7

9 years ago
Whoops, forgot to update the bug when I pushed the patch a couple of weeks ago.

Pushed changeset b4e566c1043b to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.