1125952 - 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)

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Daniel Holbert [:dholbert]]

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.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot of teeny about:sessionrestore list, in huge Firefox window

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.

Comment 2

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

Well, this is embarrassing.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot of old about:sessionrestore list (in smaller Firefox window, but still takes advantage of available space)

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.

Comment 4

[Tim Nguyen :ntim]

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)

Comment 5

[Daniel Holbert [:dholbert]]

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.)

Comment 6

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: big usability regression when trying to recover from a crash

Comment 7

[Tim Nguyen :ntim]

(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 ?

Comment 8

[Daniel Holbert [:dholbert]]

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.

Comment 9

[Mats Palmgren (inactive)]

Attachment: wip

I don't see why <div class="container"> is needed.  Can't we just give
<body> a flexbox column layout?  Something like this...

Comment 10

[Tim Nguyen :ntim]

(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.

Comment 12

[Tobias B. Besemer [:BesTo] (QA)]

Attachment: 2015-03-31 20_08_34-Restore Session - Firefox Developer Edition (Build 20150331004004).png

Screenshot

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Too late for 38 but we will take a fix for 39.

Comment 14

[garfield1234]

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

Comment 15

[garfield1234]

frame not "fame" keyboard old....

Comment 16

[JP Vossen]

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.  :-(

Comment 17

[Tim Nguyen :ntim]

Attachment: Patch

Comment 18

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

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.

Comment 19

[Tim Nguyen :ntim]

Attachment: Patch v1.1 (r=jaws)

Comment 20

[Tim Nguyen :ntim]

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

Comment 21

[Tim Nguyen :ntim]

[Tracking Requested - why for this release]: Makes about:sessionrestore harder to use.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Sorry but this is too late for 38.0.5.

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8609487 [details] [diff] [review]
Patch v1.1 (r=jaws)

Taking it for 39 as it is early in the cycle.

Comment 24

[Pulsebot]

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

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fb790e28194e

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/50bf62afd416

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0ab85a40499b

Comment 29

[Gingerbread Man]

Attachment: 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?

Comment 30

[Tim Nguyen :ntim]

(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 ?