632555 - Scale about:sessionrestore based on viewport height

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

We have media queries so we can do something to make about:sessionrestore not suck so much in a tall window.

Comment 1

[Jez Ng [:int3]]

Attachment: Patch v1

I had a go at this using flexboxes. With this patch the treeview expands such that the whole errorPageContainer takes up 70% of the total height. I thought about making the treeview expand only if there were enough treechildren entries to fill it up, but I couldn't find an easy CSS-only way to do it.

Comment 2

[Jez Ng [:int3]]

Attachment: Patch v1

Removed an extra newline.

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 549649 [details] [diff] [review]
Patch v1

Something with the spacing is wrong and it just looks off. Ultimately not a huge deal, but it's obvious enough. The min width also appears to be different (resizing the window will reflow at different widths for nightly vs this patch).

Even if all of that was good, you're missing winstripe & gnomestripe changes.

Dão, perhaps you can give some pointers on making this polished? I'm not familiar with -moz-box and how that plays with everything else.

Comment 4

[Jez Ng [:int3]]

Attachment: Patch v2

Fixed the sizing issues. A quick explanation:

width:auto doesn't cause flexboxes to expand to fill their available width; instead we should use width:-moz-available.

Vertical margins don't collapse for flexboxes. I fixed this by manually calculating the 'real' margin widths in the original about:sessionrestore and using those values instead.

I've tested on OS X and Linux; will request review once I get a look at Windows. It's the same code across platforms, though.

Comment 5

[Jez Ng [:int3]]

Attachment: Patch v2

Comment 6

[Jez Ng [:int3]]

Attachment: Screenshot of pixel imperfections

So to get a more precise comparison I took screenshots and overlaid them in Photoshop and I discovered some pixel imperfections.. probably the most significant one is that the treeview is now eight pixels narrower. Occurs on OS X and Windows, and I wouldn't be surprised if it was on Linux as well. The OS X overlaid-screenshot is attached. I think the differences are insignificant enough though.

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 550317 [details] [diff] [review]
Patch v2

Looks good to me, but I'd like to get a second pair of eyes on it just to make sure.

We could have an XP stylesheet in sessiontore/content since you're adding a fair bit more duplication, but personally, it's not a huge deal either way. I don't think we want to spend any more time on this.

Comment 8

[Dão Gottwald [::dao]]

Comment on attachment 550317 [details] [diff] [review]
Patch v2

>+#errorLongContent {
>+  display: -moz-box;
>+  -moz-box-flex: 1;
>+  -moz-box-orient: vertical;
>+}
>+
>+#errorTrailerDesc {
>+  display: -moz-box;
>+  -moz-box-flex: 1;
>+  -moz-box-orient: vertical;
> }

These two rules can be merged.

Comment 9

[Jez Ng [:int3]]

Attachment: Patch v3

Good catch.

Comment 10

[Marco Bonardo [:mak]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/4d2c4722547c

Comment 11

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/4d2c4722547c