The default bug view has changed. See this FAQ.

Scale about:sessionrestore based on viewport height

RESOLVED FIXED in Firefox 8

Status

()

Firefox
Session Restore
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: zpao, Assigned: int3)

Tracking

({polish})

Trunk
Firefox 8
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(2 attachments, 4 obsolete attachments)

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

Comment 1

6 years ago
Created attachment 549648 [details] [diff] [review]
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.
Attachment #549648 - Flags: review?(paul)
(Assignee)

Comment 2

6 years ago
Created attachment 549649 [details] [diff] [review]
Patch v1

Removed an extra newline.
Attachment #549648 - Attachment is obsolete: true
Attachment #549649 - Flags: review?(paul)
Attachment #549648 - Flags: review?(paul)
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.
Attachment #549649 - Flags: review?(paul)
Attachment #549649 - Flags: review-
Attachment #549649 - Flags: feedback?(dao)
(Assignee)

Comment 4

6 years ago
Created attachment 550316 [details] [diff] [review]
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.
Assignee: nobody → jezreel
Attachment #549649 - Attachment is obsolete: true
Attachment #549649 - Flags: feedback?(dao)
(Assignee)

Comment 5

6 years ago
Created attachment 550317 [details] [diff] [review]
Patch v2
Attachment #550316 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 550327 [details]
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.
(Assignee)

Updated

6 years ago
Attachment #550317 - Flags: review?(paul)
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.
Attachment #550317 - Flags: review?(paul)
Attachment #550317 - Flags: review?(dao)
Attachment #550317 - Flags: review+
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.
Attachment #550317 - Flags: review?(dao) → review+
(Assignee)

Comment 9

6 years ago
Created attachment 550926 [details] [diff] [review]
Patch v3

Good catch.
Attachment #550317 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/integration/mozilla-inbound/rev/4d2c4722547c
Keywords: checkin-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/4d2c4722547c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
You need to log in before you can comment on or make changes to this bug.