If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Try to keep app tabs and normal tabs together when restoring a window

NEW
Assigned to

Status

()

Firefox
Session Restore
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: andreshm)

Tracking

Trunk
x86_64
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
1. Close your only browser window, which contains both app tabs and normal tabs.
2. Click the Dock icon. (This causes Firefox to open a browser window with app tabs.)
3. History > Recently closed windows > Restore all tabs.

Result: Now my app tabs are in one window and my normal tabs are in another :(

Expected: Since I hadn't really done anything with the window created at step 2, adding the restored normal tabs to that window would be sensible.
We actually do try to keep them together. When you say "hadn't really done anything", what did you do? If it's just home pages we should overwrite them (though if a home page redirects, that breaks the rule).
(Reporter)

Comment 2

5 years ago
My home page is a local HTML file, so it shouldn't redirect.
(Assignee)

Updated

5 years ago
Assignee: nobody → andres
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 649926 [details] [diff] [review]
Patch v1

Looking at this bug, I'm not sure if this behavior was desired. 

Since, currently, when opening the new window (clicking on dock icon), first it checks if there are pinned tabs in the last closed window. If, that is the case, then the pinned tabs are moved to the new created window. The closed window info only keeps the normal tabs and if there are no normal tabs, then the closed window data is removed. 
So, when clicking on restore closed window, the pinned tabs don't exists anymore. 

The attached patch fix this, by copying the pinned tabs data but without removing the info from the closed window. So, when restoring a closed window, all info is still there.
Attachment #649926 - Flags: review?(ttaubert)
I think that's not the right fix for this. What Jesse was expecting is (given I understand him correctly) that when restoring a window it should overwrite the active one if it's empty. Also when the current window is "empty with app tabs" and the to-be-restored window has no app tabs.

Currently, undoCloseWindow() does always open a new window. Maybe we should check if there's a single window that is empty (or empty with app tabs) and use that as the restoration target.

Do you think that would make sense?
Comment on attachment 649926 [details] [diff] [review]
Patch v1

Clearing review for now.
Attachment #649926 - Flags: review?(ttaubert)
(Assignee)

Comment 6

5 years ago
(In reply to Tim Taubert [:ttaubert] from comment #4)
> I think that's not the right fix for this. What Jesse was expecting is
> (given I understand him correctly) that when restoring a window it should
> overwrite the active one if it's empty. Also when the current window is
> "empty with app tabs" and the to-be-restored window has no app tabs.
> 
> Currently, undoCloseWindow() does always open a new window. Maybe we should
> check if there's a single window that is empty (or empty with app tabs) and
> use that as the restoration target.
> 
> Do you think that would make sense?

By empty do you mean just the app tabs? Since, on this case, when clicking on the dock icon, it will open a new window with the app tabs but also with the homepage or blank page (depending on 'when browser starts' pref). So, it'll always have a normal tab. And how do we know the normal tab is not loaded or created by the user before restoring the closing window? Or if we have more than one homepage, then more than one tab is opened?
(Assignee)

Updated

5 years ago
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.