Closed Bug 1267501 Opened 8 years ago Closed 8 years ago

New Private Browsing start-page overflows off the *left side of the window* (making content unscrollable) for small window sizes

Categories

(Firefox :: Private Browsing, defect, P1)

48 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 49
Iteration:
49.1 - May 9
Tracking Status
firefox47 --- unaffected
firefox48 + fixed
firefox49 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Keywords: regression, Whiteboard: [fxprivacy])

Attachments

(1 file)

STR:
 1. Open a new private browsing window.
 2. Resize the window to be skinny, say 300-400px wide.
 3. Try to scroll around horizontally to read the page's contents (using the scrollbars).

ACTUAL RESULTS:
- If you scroll all the way to the left, you'll see that the page's contents overflow off the left side of the viewport, to the extent that they're unscrollable and hence unreadable.
- If you scroll all the way to the right, you'll see that the page's background-color ends abruptly, and some text protrudes past that.


EXPECTED RESULTS:
* Contents should be scrollable/readable.
* No awkward background-color-ending in the region of the viewport that is scrollable.
Blocks: 1216897
Whiteboard: [fxprivacy] [triage]
What's happening here is:
 (1) The body has "display:flex; flex-direction: column; align-items: center". So its children are centered, in the left-to-right axis.

 (2) The flex item has a child with an *explicit width* of 780px, here:
> 54 .about-content-container {
> 55   width: 780px;
> 56 }
http://hg.mozilla.org/mozilla-central/annotate/79de998e7307/browser/themes/shared/privatebrowsing/aboutPrivateBrowsing.css#l54

So, since the body is skinnier than 780px, that element *MUST* overflow, and it overflows in equal parts on the left and the right (since it's centered with align-items).

 (3) Only the overflow on the bottom and right are scrollable, as defined by CSS here:
    https://www.w3.org/TR/cssom-view-1/#scrolling-area


I think we should absolutely remove the "width: 780px" there -- that seems bogus, and it doesn't seem to be present in the mockup of this feature (at http://people.mozilla.org/~bbell/about-page-improvements/about-privateBrowsing.html ).  Maybe "max-width: 780px" is what we really want?
Flags: needinfo?(rchien)
(I'll just take this bug, since I've already got a fix and I think it's correct, or at least more correct than what we've got at the moment. There's more to be done for small window-sizes, as described in bug 1267499 -- but with this fix, all the content should end up being visible via scrolling at least.)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Flags: needinfo?(rchien)
See Also: → 1267499
I'd prefer to deal with bug 1267047 first, which I expect will likely fix this anyway.
Is that bug going to remove the line with the hardcoded "width" here?

If so, that sounds good to me.
(If not, then I still think we should remove that line, or adjust it to be a max-width constraint. Otherwise, to the extent that it has any effect, it's a footgun for skinny windows.)
Priority: -- → P3
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Comment on attachment 8745147 [details]
MozReview Request: Bug 1267501: Adjust hardcoded 'width' in new Private Browsing UI to be a max-width instead, so it fits better in skinny windows. r?rickychien

https://reviewboard.mozilla.org/r/48877/#review46007

Looks good to me on my local machine. thanks!
Attachment #8745147 - Flags: review?(rchien) → review+
Thanks, Ricky!

Gijs, are you still opposed to taking this one-liner, or is OK for me to land this?  Per comment 4, I think this is strictly an improvement, and I expect it should be independent of other work.

(Other bugs are still free to remove this line as-needed, if that's what you were saying is going to happen (not sure).  But in the meantime, we're better off with "max-width" instead of "width" here, from a responsiveness perspective.)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Daniel Holbert [:dholbert] from comment #7)
> Thanks, Ricky!
> 
> Gijs, are you still opposed to taking this one-liner, or is OK for me to
> land this?  Per comment 4, I think this is strictly an improvement, and I
> expect it should be independent of other work.
> 
> (Other bugs are still free to remove this line as-needed, if that's what you
> were saying is going to happen (not sure).  But in the meantime, we're
> better off with "max-width" instead of "width" here, from a responsiveness
> perspective.)

We can take this for now, but more extensive work will likely need to be done.
Flags: needinfo?(gijskruitbosch+bugs)
https://hg.mozilla.org/mozilla-central/rev/c164e1eed334
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Iteration: --- → 49.1 - May 9
Flags: qe-verify?
Priority: P3 → P1
Depends on: 1269485
No longer depends on: 1269485
Comment on attachment 8745147 [details]
MozReview Request: Bug 1267501: Adjust hardcoded 'width' in new Private Browsing UI to be a max-width instead, so it fits better in skinny windows. r?rickychien

Approval Request Comment
[Feature/regressing bug #]: bug 1259340
[User impact if declined]: in narrow windows, horizontal scrollbars show up for about:privatebrowsing. This small 1-line patch also enables landing some of the other regression fixes that we made since landing bug 1259340.
[Describe test coverage new/current, TreeHerder]: no, this is a styling issue.
[Risks and why]: low risk, one-line change
[String/UUID change made/needed]: no
Attachment #8745147 - Flags: approval-mozilla-aurora?
Recent regression, tracking
Comment on attachment 8745147 [details]
MozReview Request: Bug 1267501: Adjust hardcoded 'width' in new Private Browsing UI to be a max-width instead, so it fits better in skinny windows. r?rickychien

Fix for minor recent regression, fine to uplift!
Attachment #8745147 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Version: unspecified → 48 Branch
You need to log in before you can comment on or make changes to this bug.