1128237 - In-content preferences SubDialog is cut off on WXGA(1366x768) monitor

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Alice0775 White]

Attachment: screenshot

In-content preferences SubDialog is cut off on WXGA(1366x768) monitor of cheaper Notebook computer.

WXGA(1366x768) is common monitor size for cheaper Notebook computer.


Steps to reproduce:
0. Setup ubuntu on WXGA(1366x768) monitor of cheaper PC Notebook computer.
1. Login with GNOME Classic
2. Start Browser and enable MenuBar and Bookmarks ToolBar
3. Open In-content preferences > Privacy > Show Cookies

Actual Results:
In-content preferences SubDialog is cut off.

Expected Results:
Should not cut off.

Comment 1

[:Gijs (he/him)]

Yoink.

This is reproducible on other OSes by just making the window smaller. I would have expected the richlistbox to resize and the dialog to be smaller, but it doesn't. Not sure how easy it'll be to fix this, esp. because the same XUL is used non-incontent, but will be having a look.

Comment 2

[Ian Moody [:Kwan] (UTC+1)]

I probably should have filed this earlier.  It was biting me the other week while mucking around with in-content prefs since I'd shrunk the window down.
At the time I had no luck fixing it, since giving #diaglogBox a max-height: 100vh and overflow (my favourite trick it appears) didn't help and instead just moved it down the page for no reason I could comprehend. I tried mucking with the XUL box stacking/flexing to no avail.
However I just went back and played and just
#dialogOverlay {
  max-height: 100vh;
  overflow: auto;
}
at least makes it usable, though aesthetically I think it leaves something to be desired, since the scrolling is in the window rather than just the subDialog, which is what I'd been going for.

Comment 3

[:Gijs (he/him)]

Attachment: fix dialogs on small screen sizes,

This goes after your patch for bug 1044597. It's the least sucky solution I could think of, even if it's not ideal - overflow on the XUL window element doesn't seem to work, and if we hardcode overflow for an element inside the dialog, then the dialog's scrollWidth is bogus. So I'm assigning it if the inner window height is bad when the dialog is opened. I realize this means we don't adjust for windows that are resized later, but in that case I guess I'm OK with the onus being on the user to resize the window back to being large again.

Comment 4

[:Gijs (he/him)]

For the sake of the stupid translations.xul file, here's a diff -w (going to be gone in 1 month, but I figure your average review times are well below that ;-) ):

http://pastebin.mozilla.org/8590264

Comment 5

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #3)
> dialog, then the dialog's scrollWidth is bogus.

Clearly I meant scrollHeight. Time for sleep!

Comment 6

[:Gijs (he/him)]

(fwiw, this patch depends on yours in bug 1044597 )

Comment 7

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

Comment on attachment 8560782 [details] [diff] [review]
fix dialogs on small screen sizes,

Review of attachment 8560782 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the -w diff.

::: browser/themes/shared/incontentprefs/preferences.inc.css
@@ +350,5 @@
>    width: 66ch;
>  }
>  
> +.largeDialogContainer.doScroll {
> +  overflow-y: auto;

Setting overflow-y to auto will also result in overflow-x being set to auto. If you want overflow-x to not get `auto` then you will need to explicitly set `overflow-x: visible;`

Comment 8

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Comment on attachment 8560782 [details] [diff] [review]
> fix dialogs on small screen sizes,
> 
> Review of attachment 8560782 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the -w diff.
> 
> ::: browser/themes/shared/incontentprefs/preferences.inc.css
> @@ +350,5 @@
> >    width: 66ch;
> >  }
> >  
> > +.largeDialogContainer.doScroll {
> > +  overflow-y: auto;
> 
> Setting overflow-y to auto will also result in overflow-x being set to auto.
> If you want overflow-x to not get `auto` then you will need to explicitly
> set `overflow-x: visible;`

This won't work: http://stackoverflow.com/a/6433475/713326

I don't worry about this too much because XUL. Should I? (in particular, do you observer horizontal scroll anywhere with this patch + the one in bug 1044597?)

It could be worked around with a wrapper element, AIUI... but I'm reluctant to do that because of the above. Thoughts?

Comment 9

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

I didn't observe a horizontal scrollbar with these patches, but I thought I had seen an issue like that before.

We can land this and keep an eye out for this or any bug reports of people seeing horizontal scrollbars.

Comment 10

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/1900ac4cdf15

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1900ac4cdf15

Comment 12

[Camelia Badau [:cbadau], Desktop QA]

Attachment: ChangeMasterPassword.png

Verified on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 38 Beta 8 (buildID: 20150427090451) and I have 2 situations: 

1. Make the window smaller -> open one subdialog (e.g: Security tab -> Change Master Password) => the subdialog is cut off and the buttons aren't visible; no horizontal/vertical scrollbar displayed (see attachment "ChangeMasterPassword.png")

2. Launch Firefox -> Open about:preferences -> Open one subdialog (e.g: Privacy tab -> Cookies) -> make the window smaller => the subdialog is cut off and no horizontal/vertical scrollbar is displayed (see attachment "Cookies.png")

What do you think about this?

Comment 13

[Camelia Badau [:cbadau], Desktop QA]

Attachment: Cookies.png

Comment 14

[:Gijs (he/him)]

These are really very small sizes (both screenshots have a window that's about 500px high), and neither dialog was made for those sizes. Please file a followup bug for this.

Comment 15

[Camelia Badau [:cbadau], Desktop QA]

(In reply to :Gijs Kruitbosch from comment #14)
> These are really very small sizes (both screenshots have a window that's
> about 500px high), and neither dialog was made for those sizes. Please file
> a followup bug for this.

Thank you for your answer! 
I filled bug 1159233. 

Marking this bug as VERIFIED FIXED.