Closed Bug 1125952 Opened 10 years ago Closed 9 years ago

New about:sessionrestore UI's list of tabs to restore is too small & doesn't grow to accomodate window size (impossible to keep your place when scrolling through it)

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Version:
38 Branch
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 41
Tracking Flags:
Tracking Status
firefox38 + wontfix
firefox38.0.5 - wontfix
firefox39 + fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: dholbert, Assigned: ntim)

References

(Depends on 1 open bug,
URL
)

Details

(Keywords: regression)

Attachments

(5 files, 2 obsolete files)

screenshot of teeny about:sessionrestore list, in huge Firefox window
(deleted), image/png
Details
screenshot of old about:sessionrestore list (in smaller Firefox window, but still takes advantage of available space)
(deleted), image/png
Details
2015-03-31 20_08_34-Restore Session - Firefox Developer Edition (Build 20150331004004).png
(deleted), image/png
Details
Patch v1.1 (r=jaws)
(deleted), patch
ntim
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Sylvestre
: approval-mozilla-release-
Details | Diff | Splinter Review
Nightly 2015-05-27 before and after.png
(deleted), image/png
Details
STR: 1. Trigger about:sessionrestore with a bunch of tabs to be restored. (e.g. by crashing via bug 1125304 comment 2, and then restarting firefox.) (I don't exactly know how to *reliably* make a crash trigger about:sessionrestore on the next run, but setting browser.sessionstore.max_resumed_crashes to 0 seems to help.) 2. Resize your window to be large. Try to scroll through the list of restored tabs. ACTUAL RESULTS: - The list of restored tabs stays the same size (5 lines tall), regardless of window size. - When you scroll with your mousewheel, 3 lines are scrolled at a time -- so given the fixed 5-line height, this means the majority of the pane is *completely new* with each scrollwheel tick. This makes it impossible to keep track of your place when scrolling with a mousewheel. EXPECTED RESULTS: - The list of restored tabs should grow to take advantage of the extra space when your window is large. - It should also perhaps start out larger. (5 lines is not very tall, particularly given that one of the lines is occupied by "Window 1") - (The 3-lines-at-a-time scrolling thing should also perhaps be reduced, but that's an issue that predates this new UI, and I filed bug 1125926 to cover that.)
Here's what this UI looks like on my fullscreen Firefox window (on a monitor with 1900x1200 resolution). There's so much wasted space, and so many tabs crammed into a tiny list.
Well, this is embarrassing.
Here's a screenshot of the old UI. This window isn't even fullscreen, but the fact that it grows to match the window-size (combined with the smaller text) means it shows me many more of my saved tabs. It's much easier to scroll through, too, because each scroll-tick doesn't blow away the majority of the scroll area. See also screencasts on bug 1125926 to see what scrolling with a mousewheel is like in the new & old UI.
Attachment #8554695 - Attachment description: screenshot of old about:sessionrestore list → screenshot of old about:sessionrestore list (in smaller Firefox window, but still takes advantage of available space)
I've been experimenting with 2 options : - Increase the tree min-height http://imgur.com/a/6qfne Top screen is 20em, and bottom screen is 16em - Decrease font size I also tried decreasing the font-size to 14px. http://i.imgur.com/2Uuvbma.png (with current min-height) http://i.imgur.com/r9JovcA.png (with min-height: 16em)
Thanks for jumping on this! Though -- hmm, neither of those options you mentioned (increase min-height, decrease font-size) seem responsive to window-height (which the old design was). Is it possible to make the tree height grow as the window grows? That's really the main thing I was hoping for, when filing this bug. As I understand it, the goal of this UI is to address two use-cases: (1) Help the user find the one tab that's likely to have caused the crash (so they can restore everything but that tab, & hopefully not crash again) (2) Help the user find the few tabs that they care about restoring (so they can trash everything else and start near-fresh, & hopefully not crash again) In both use-cases, it seems like the user isn't well served if we artificially cap the number of tabs we show them (and let blank page-space go to waste). That only makes it harder for them to skim through their tab-list, and makes it more likely they'll miss something.)
[Tracking Requested - why for this release]: big usability regression when trying to recover from a crash
tracking-firefox38: --- → ?
(In reply to Daniel Holbert [:dholbert] from comment #5) > Thanks for jumping on this! Though -- hmm, neither of those options you > mentioned (increase min-height, decrease font-size) seem responsive to > window-height (which the old design was). Is it possible to make the tree > height grow as the window grows? That's really the main thing I was hoping > for, when filing this bug. I was trying to do this, but I couldn't get the sessionrestore box to flex without limiting the height. I'm not sure why. Maybe that's because xul is used with xhtml ?
So I don't know what you've tried already -- but to get things to grow to fit the window-size, you need things to be either percentage-sized or flexible, all the way up the DOM. So, e.g. right now, .container is sized to its contents (and then centered vertically in the <body> via the body's "align-items:center"). In order for .container to resize to fit the window, you need to either: (1) Give its parent (<body>) an explicit "height" (e.g. change min-height:100vh to just height:100vh), and give .container a percent-height ...OR: (2) Remove "align-items:center" from <body> (which will then leave it with the default "align-items:stretch", which forces its child (.container) to stretch its height to match <body>. You need to do something like that at each level, down to the tree itself.
OS: Linux → All
Attached patch wip (obsolete) (deleted) — — Splinter Review
I don't see why <div class="container"> is needed. Can't we just give <body> a flexbox column layout? Something like this...
(In reply to Mats Palmgren (:mats) from comment #9) > Created attachment 8557402 [details] [diff] [review] > wip > > I don't see why <div class="container"> is needed. Can't we just give > <body> a flexbox column layout? Something like this... The stylesheet is also used for about:tabcrashed and about:welcomeback. We wouldn't want those pages having their content stretched to the full height. (In reply to Daniel Holbert [:dholbert] from comment #8) Thanks for your help, I should be able to attach a patch by next week.
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
Assignee: ntim007 → nobody
Status: ASSIGNED → NEW
Severity: normal → major
status-firefox38: --- → affected
status-firefox39: --- → affected
Component: General → Session Restore
Hardware: x86_64 → All
Version: Trunk → 38 Branch
Too late for 38 but we will take a fix for 39.
status-firefox38: affected → wontfix
tracking-firefox39: --- → +
Maybe is good to consider the option of an dragging spot (lower in the right corner...) to resize with the mouse, like in "Comment" form fields of web pages on "Contact" ( as this fame for Comment in fact I used to post this... :) ) http://imgur.com/mYfVxvo
frame not "fame" keyboard old....
FWIW I strongly agree with OP, this behavior is bad. A much bigger window is better, but a resizable one would work too. Use case: I keep a lot of windows and tabs open, and they get out of control, so I sometimes `killall firefox` then use the handy "Restore Session" box to see at a glance what I had, and close out obsolete stuff. 5 lines of "at a glance" does NOT work. :-(
Attached patch Patch (obsolete) (deleted) — — Splinter Review
Attachment #8557402 - Attachment is obsolete: true
Attachment #8608255 - Flags: review?(jaws)
Comment on attachment 8608255 [details] [diff] [review] Patch Review of attachment 8608255 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the width:100% mentioned below removed. Thanks! ::: toolkit/themes/shared/in-content/info-pages.inc.css @@ +92,5 @@ > /* Trees */ > +.tree-container { > + margin-top: 1.2em; > + flex-grow: 1; > + width: 100%; width:100% isn't necessary here. The width rule on the `tree` two rulesets below covers this.
Attachment #8608255 - Flags: review?(jaws) → review+
Attached patch Patch v1.1 (r=jaws) (deleted) — — Splinter Review
Assignee: nobody → ntim.bugs
Attachment #8608255 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8609487 - Flags: review+
Keywords: checkin-needed
Comment on attachment 8609487 [details] [diff] [review] Patch v1.1 (r=jaws) Approval Request Comment (release request is for 38.0.5) [Feature/regressing bug #]: bug 1087618 (which landed in 38) [User impact if declined]: Hard to scroll in tab list [Describe test coverage new/current, TreeHerder]: Will land on m-c, tested locally [Risks and why]: Pretty low, CSS only patch [String/UUID change made/needed]: Nope
Attachment #8609487 - Flags: approval-mozilla-release?
Attachment #8609487 - Flags: approval-mozilla-beta?
Attachment #8609487 - Flags: approval-mozilla-aurora?
[Tracking Requested - why for this release]: Makes about:sessionrestore harder to use.
tracking-firefox38.0.5: --- → ?
Sorry but this is too late for 38.0.5.
status-firefox38.0.5: --- → wontfix
status-firefox40: --- → affected
status-firefox41: --- → affected
tracking-firefox38.0.5: ? → -
Comment on attachment 8609487 [details] [diff] [review] Patch v1.1 (r=jaws) Taking it for 39 as it is early in the cycle.
Attachment #8609487 - Flags: approval-mozilla-release?
Attachment #8609487 - Flags: approval-mozilla-release-
Attachment #8609487 - Flags: approval-mozilla-beta?
Attachment #8609487 - Flags: approval-mozilla-beta+
Attachment #8609487 - Flags: approval-mozilla-aurora?
Attachment #8609487 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/0ab85a40499b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Mozilla/5.0 (Windows NT 5.1; rv:41.0) Gecko/20100101 Firefox/41.0 Build ID: 20150527135446 Windows XP, 1024×768 resolution. There's virtually no difference. The list went from 4 items to 5½. Is this really considered fixed?
(In reply to Gingerbread Man from comment #29) > Created attachment 8612076 [details] > Nightly 2015-05-27 before and after.png > > Mozilla/5.0 (Windows NT 5.1; rv:41.0) Gecko/20100101 Firefox/41.0 > Build ID: 20150527135446 > > Windows XP, 1024×768 resolution. There's virtually no difference. The list > went from 4 items to 5½. Is this really considered fixed? It now flexes to your window height, of course there's still some padding at the top and at the bottom to leave breathing space. I could reduce that padding though. I could also reduce the tree font size. Can you file a follow up about optimizing things further ?
Depends on: 1169529
Depends on: 1172357
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: