868495 - Browser main preference pane cut off on Windows 7 with hardware acceleration enabled

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[rsx11m]

Observed in bug 867210 already for a different pref pane, but in this case the entire button appears to be cut off at the bottom (screenshot follows). Interestingly, this only happens with hardware acceleration enabled, thus I'm wondering if something broader is going on with calculating the "em" sizes.

Since the pref-pane height remains 530px regardless of the acceleration setting, it appears to me that XUL doesn't account correctly for any changes in font size induced by hardware acceleration when calculating font heights and maps the "em" unit to actual pixels. Thus, this may be in fact a Core issue rather than isolated to SeaMonkey.

Comment 1

[rsx11m]

Attachment: Screenshot

The "Restore Default" button is almost no longer accessible, but only when hardware acceleration is enabled. If switched off, it fits nicely.

Using the workaround mentioned in bug 867210 comment #5 and setting prefWindow.size to "width: 52em; height: 43em;" (from 41em) resolves the problem.

Comment 2

[rsx11m]

Make that 44em on Windows Classic with hardware acceleration, which seems to require even more space than the Windows 7 desktop theme, otherwise the lowest button and the bottom border of lowest button is still cut off.

The preferences window isn't the only dialog affected. When setting up a new e-mail account with the Account Wizard, the second dialog (Incoming Server Information) is cut off for a POP account (but only with the Windows Classic, not the Windows 7 default desktop theme). In contrast to the preferences, a scrollbar appears here, thus the "Use Global Inbox" checkbox remains accessible.

This makes me thinking that the issue really should be solved at the XUL/CSS/layout level. If we increase the height for Windows to 44em based on what I've measured here but leave it 41em for the other platforms, someone may use a configuration not needing the additional space, fills it up with a new UI element, and then yet another too-long case occurs for the other platforms or an affected Windows configuration.

Comment 3

[rsx11m]

(In reply to rsx11m from comment #2)
> lowest button and the bottom border of lowest button is still cut off.
                                         ^^^^^^^^^^^^^
I meant the textbox for the home-page group here...

Comment 4

[[e-mail]]

(In reply to rsx11m)
> Since the pref-pane height remains 530px regardless of the acceleration
> setting, it appears to me that XUL doesn't account correctly for any changes
> in font size induced by hardware acceleration when calculating font heights
> and maps the "em" unit to actual pixels. Thus, this may be in fact a Core
> issue rather than isolated to SeaMonkey.

That's... surprising. Unfortunately I can only run Windows 7 in a VirtualBox VM where we don't support graphics acceleration. Please post a comparison screenshot (preferably with multiple home pages, so I can see whether there is any affect on the multiline textbox). Thanks!

Comment 5

[rsx11m]

Attachment: Comparison with 43em height

Windows 7 default theme, style overridden from height: 41em to 43em applied. First image with hardware acceleration disabled, second image with hardware acceleration enabled.

Except for the Category picker on the left-hand side (and maybe the "Browser" banner on the top), it appears to me that all other items require slightly more height with hardware acceleration enabled. This includes the home-button textbox, which itself becomes about 9px larger.

Comment 6

[Phoenix]

Problem is also present in "Browser" - "Location Bar" and "Privacy & Security" sections

Comment 7

[rsx11m]

I found layout bug 657864 - rounding errors in (normal) line heights from font metrics - talking about slight differences in height calculations depending on how the fonts are rendered. Maybe that's contributing to what we are seeing here?

Comment 8

[Jonathan Kew [:jfkthame]]

Although it's certainly possible that bug 657864 is a factor here, it looks to me like there's also a more general issue with how this dialog is being laid out - it seems to be making assumptions about font sizes and spacing that may not hold across platforms, versions, themes, etc. What happens if the user's Windows theme (or whatever) sets a UI font that has an inherently larger default line-height?

Note that the "em" unit will map to a fixed number of pixels based on font-size, but the default line height of lines of text may vary depending on the font (unless explicitly controlled via css). Thus, assuming that up to N lines of text can be displayed within a height of X em is a fragile assumption.

Comment 9

[Jonathan Kew [:jfkthame]]

(In reply to [e-mail] from comment #4)

> That's... surprising. Unfortunately I can only run Windows 7 in a VirtualBox
> VM where we don't support graphics acceleration.

FWIW, you could probably reproduce the issue in a Win7 VM without hardware acceleration by setting gfx.font_rendering.directwrite.enabled to true. This should force the use of DirectWrite font code, even with unaccelerated GDI graphics.

Comment 10

[rsx11m]

(In reply to Jonathan Kew (:jfkthame) from comment #8)
> Note that the "em" unit will map to a fixed number of pixels based on
> font-size, but the default line height of lines of text may vary depending on
> the font (unless explicitly controlled via css). Thus, assuming that up to N
> lines of text can be displayed within a height of X em is a fragile assumption.

Jonathan, thanks for your comments. Seeing that prefpanes tend to get crowded, so far the assumption was that the line height expressed in em is a relative constant to figure out how many elements would fit. I was under the impression that "em" as a unit reflects a fraction of that line height, so apparently I was wrong.

Thus, it seems that the options here are to either freeze the line height to an explicit em value, thus elements are cropped individually (if they are) rather than the pane cut off at the bottom (no clue what can of worms that may open).

The other option is to increase prefWindow.size, but then for all platforms, and watch in reviews that nobody fills up that safety margin too much.

(In reply to Jonathan Kew (:jfkthame) from comment #9)
> FWIW, you could probably reproduce the issue in a Win7 VM without hardware
> acceleration by setting gfx.font_rendering.directwrite.enabled to true.

Along with layers.acceleration.disabled set to "true" that gives me the same line increase, so that's a good workaround for testing. The Windows Classic desktop theme gives me the largest line-height increase.

Comment 11

[[e-mail]]

Interestingly the effect appears to depend on the font; I copied DejaVu Sans from my Linux box and that has the same line height with or without directwrite font rendering.

The rendering of the default font matches Notepad only without directwrite font rendering. This could be because Notepad doesn't use directwrite but I don't know of any other applications that do.

Comment 12

[rsx11m]

Since this has stalled a bit, I'll go ahead an throw in a few options. Obviously, it doesn't leave a good first impression if the default settings cause SeaMonkey's own panes to be cut off, and barely being able to see or reach a button may cause usability issues, thus I think that /something/ has to be done to fix this before the next merge is due, preferably all the way down to the comm-beta channel.

We could go for an interim fix, either tackling the issue at the source and disabling DirectWrite fonts altogether, or go with an expansion of the prefWindow height, while looking for a more general solution to the issue.

Comment 13

[rsx11m]

Attachment: Option A: Disable Direct2D

This first option disables Direct2D by default, thus leaving the historical line heights intact without the need to extend the window height. Even though not disabling hardware acceleration as such, unfortunately this would also affect font rendering in content, thus may not be the preferred solution. On the other hand, it would also fix other window-height issues as stated in comment #2. 

Jonathan, is there a more specific preference to disable DirectWrite fonts in chrome?

Comment 14

[rsx11m]

Attachment: Option B: 43em, all platforms

The second option moderately increases the prefWindow height by 2em to the 43em measured necessary for the Windows 7 default desktop theme with hardware acceleration enabled (this would still leave a mild cut-off issue with Windows Classic, but not impacting usability in any way). For consistency, this is done on all platforms.

A similar approach was taken for the Account Settings window, which initially had a height of 44em, increased to 50em by bug 218902, then again to 55em by bug 688694. At 43em, the Preferences window would still be much smaller than the Account Settings (i.e., if 55em is good for everybody, 43em should be as well), so this seems to be a moderate change.

The problem stated in comment #10 is that further additions/modifications on non-affected platforms (or when tested without DirectWrite fonts applied) may fill up the space, then causing again a cut off on Windows with DirectWrite fonts. In essence, each patch will have to be verified against the platform and settings using the most space.

Comment 15

[rsx11m]

Attachment: Option C: 43em, Windows only

This third alternative is a reduced version of attachment 753776 [details] [diff] [review], only applying the change to Windows under the assumption that only Direct2D (which is Windows-specific) exhibits the problem sufficiently to justify adjusting the prefWindow height. At least with my Linux installation, that assumption holds, and Stefan would need to confirm that the current height is ok on Mac OSX as well.

This would reduce the effort of controlling window height to Windows platform, where patches still would have to be tested against DirectWrite-enabled settings, but patches tested on Linux and Mac would hopefully not break height restrictions on Windows.

Any comments or suggestions for other options?

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to rsx11m from comment #13)
> Created attachment 753774 [details] [diff] [review]
> Option A: Disable Direct2D
> 
> This first option disables Direct2D by default, thus leaving the historical
> line heights intact without the need to extend the window height. Even
> though not disabling hardware acceleration as such, unfortunately this would
> also affect font rendering in content, thus may not be the preferred
> solution. On the other hand, it would also fix other window-height issues as
> stated in comment #2. 
> 
> Jonathan, is there a more specific preference to disable DirectWrite fonts
> in chrome?

I don't think so. If Direct2D is used, that always implies the use of the DirectWrite font system. We can't render GDI fonts to Direct2D. And we can't render chrome via the GDI backend while using D2D for content.

(In reply to rsx11m from comment #14)
> In essence, each patch will have to be verified against
> the platform and settings using the most space.

Isn't that true in general, anyway? It's not safe to assume that text metrics (or the sizes of various other UI elements, for that matter) will be identical across platforms, so if you have a fixed-size window and fill it fairly full, it would be wise to check that things still fit as expected on all platforms. The line-height discrepancy between the GDI and DirectWrite font backends may be one of the more glaring issues, but it's also possible that horizontal text metrics will differ such that you get different line-breaks in a given block of text, and perhaps an additional line of text on one or other platform.

(Surely there are similar issues with regard to localization, too; if you have dialogs that are full of controls and text in the en-US version, it's very likely that once they're translated into other languages, things won't fit in the same way. Flexibly-sized UI elements have a lot to recommend them.)

Comment 17

[rsx11m]

(In reply to Jonathan Kew (:jfkthame) from comment #16)
> Isn't that true in general, anyway? It's not safe to assume that text
> metrics (or the sizes of various other UI elements, for that matter) will be
> identical across platforms, so if you have a fixed-size window and fill it
> fairly full, it would be wise to check that things still fit as expected on
> all platforms.

True, the introduction of the DirectWrite fonts lead to an increased significance of this issue though as they apparently differ substantially in their metrics from the other font systems, thus adding another parameter to test changes against. Also, keep in mind that many people contributing patches here don't have access to all platforms (and Mac gets frequently "forgotten" in this way), thus it's tricky to easily accomplish that.

Ideally, the dialogs would expand automatically to the extent needed to accommodate the content of the longest pane (though this would invite contributors to fill up space excessively if there doesn't seem to be a limit).

Comment 18

[rsx11m]

(In reply to rsx11m from comment #2)
> e-mail account with the Account Wizard, the second dialog (Incoming Server
> Information) is cut off for a POP account (...). In contrast to the
> preferences, a scrollbar appears here, thus the "Use Global Inbox" checkbox
> remains accessible.

Option D: I've tried a couple of variants using "overflow-y: auto;" but didn't manage to get a scroll bar with 41em. Anyway, I think it would look ugly, thus won't investigate further (at least not as an isolated solution).

(In reply to rsx11m from comment #17)
> Ideally, the dialogs would expand automatically to the extent needed to
> accommodate the content of the longest pane (though this would invite
> contributors to fill up space excessively if there doesn't seem to be a
> limit).

Option E: Replacing "height: 41em;" with "min-height: 41em;" shows the intended behavior when opening Edit > Preferences from the browser window. Opening it from the Mail & News window opens it at 41em, fitting its main preference pane; when switching to the Browser pane from there, it stays at 41em, thus won't adjust to fit the larger content (i.e., not a directly feasible solution either).

Comment 19

[[e-mail]]

(In reply to Jonathan Kew from comment #16)
> it's also possible > that horizontal text metrics will differ
The width is in ch units, which admittedly is not perfect either (things were worse when the width was in em units too though).

> (Surely there are similar issues with regard to localization, too; if you
> have dialogs that are full of controls and text in the en-US version, it's
> very likely that once they're translated into other languages, things won't
> fit in the same way. Flexibly-sized UI elements have a lot to recommend
> them.)
This is why the width and height are themselves localisable.

(In reply to rsx11m from comment #17)
> Ideally, the dialogs would expand automatically to the extent needed to
> accommodate the content of the longest pane (though this would invite
> contributors to fill up space excessively)
Which is a problem we'd like to avoid, although it would at least be fairly obvious if the prefwindow only expanded when the appropriate pane was shown.

Comment 20

[[e-mail]]

(I'm curious to know what happens if you have a multiple monitor or remote desktop setup and your window moves from an accelerated to an unaccelerated graphics card - does the line height change then?)

Comment 21

[rsx11m]

(In reply to [e-mail] from comment #19)
> The width is in ch units, which admittedly is not perfect either

That's true for the Account Settings, but the Preferences window has the width specified in em as well (see the patches).

Comment 22

[Stefan [:stefanh]]

> Stefan would need to confirm
> that the current height is ok on Mac OSX as well.

Current height is ok on Mac (I checked a nightly from May 8 since I'm not on my dev machine atm).

Comment 23

[[e-mail]]

(In reply to rsx11m from comment #21)
> (In reply to comment #19)
> > The width is in ch units, which admittedly is not perfect either
> 
> That's true for the Account Settings, but the Preferences window has the
> width specified in em as well

Oops. It almost happened in bug 394522, but we couldn't agree on a value...

Comment 24

[Phoenix]

(In reply to [e-mail] from comment #20)
> (I'm curious to know what happens if you have a multiple monitor or remote
> desktop setup and your window moves from an accelerated to an unaccelerated
> graphics card - does the line height change then?)
I can test this in a week or two, when my new hdmi cable will arrive and I could downgrade second card video driver to make it blocklisted

Comment 25

[rsx11m]

(In reply to [e-mail] from comment #23)
> Oops. It almost happened in bug 394522, but we couldn't agree on a value...

Trying the "width: 72ch;" discussed in bug 394522 comment #92 is way too narrow in Windows and gives me just 448px width with Windows 7 default theme and Direct2D enabled. To get to the 640px measured with the current value, I'd have to set it to 103ch instead (and that's close to the value for accountManager.size which yields a similar width on Windows, but not on Linux where the Account Settings appear much wider than the Preferences window).

Thus, going from em to ch metrics doesn't seem to solve much either with the values being so different among platforms.

Comment 26

[rsx11m]

Attachment: Option C: Windows only, with instructions

Since nobody seems to have further suggestions or any strong preference for solving this issue one way or the other, I'm going to make a pick.

It appears to me that Option C to just increase the height to 43em on the affected platform (Windows) is the least intrusive way to fix this for now. There is a precedence with bug 199990 (Mac OSX), which increased the width of the pane to account for wider buttons on that platform. Thus, I think that this step is justifiable. I'm adding though a comment, both to the Preferences and the Account Settings ".size" definitions, that height estimates on Windows should be tested against the larger DirectWrite fonts per attachment 745564 [details], along with comment #9 and comment #16.

On the long run, the interim fix may well be the permanent one. Based on the discussion here, I'm not sure if a proper metrics to accurately account for text and XUL element sizes with the aim to get stable and predictable numbers for all platforms, desktop themes, and font settings is feasible to start with (and if so, it would have to be solved on XUL/CSS level but not on the application level). The only other option seems to be limiting the heights of specific XUL elements, thus if they are getting too big, at least the cutoff would be equally distributed and elements should still remain accessible. But, identifying general rules and values how to define those heights seems to be a non-trivial task, and I wouldn't know how to approach this.

Comment 27

[[e-mail]]

Attachment: Possible patch

So, I finally got around to hacking something together.

The outerHeight adjustment on its own works when selecting a pref panel that is too tall, but does not work for the initial panel, because the call to sizeToContent that is needed to adjust the width correctly ignores the content size because of the overflow styles.

Overriding the overflow styles fixes that problem, but then ends up breaking the -preferences command line option, because that doesn't currently load a panel by default, so I ended up fixing that bug too.

Comment 28

[rsx11m]

Attachment: Screenshot attachment 757161

Hmm, this doesn't quite fix it for me. In fact, it looks somewhat similar to what I've seen with just using "min-height: 41em;" as a static rule.

 - left: Opening the pref pane from a browser window, adjusts fine >41em;
 - center: Opening from a mail & news window, opens at 41em;
 - right: But then, moving to the Browser preferences, stays at 41em.

In addition, when doing this with the Windows Classic desktop theme, the chrome fonts appear to shrink and look scaled while the buttons are still cut off.

Comment 29

[rsx11m]

Attachment: Screenshot Windows Classic

Actually, the vertical spacing seems to be the same, but the fonts somehow got squeezed when switching from the opening Mail & News to the Browser prefs.

Comment 30

[[e-mail]]

Hmm, I'd only tried it under Luna; I guess I should try it with other themes.

Comment 31

[Philip Chee]

Comment on attachment 757161 [details] [diff] [review]
Possible patch

Windows Classic Theme + SeaMonkey Default:

1. Open from MailNews. Prefpane opens to fit content.
2. Select Browser.
3. Prefwindow expands to fit content.

1. Open from MailNews. Prefpane opens to fit content.
2. Select Privacy & Security, then Cookies or SSL.
3. Content is pushed down past the bottom and the OK/Cancel/Help buttons are half hidden.

1. Open from MailNews. Prefpane opens to fit content.
2. Select Advanced, then Mouse Wheel.
3. Prefwindow expands to fit content.

Windows 7 default Theme + SeaMonkey Default:

1. Open from MailNews. Prefpane opens to fit content.
2. Select Browser.
3. Prefwindow expands to fit content.

1. Open from MailNews. Prefpane opens to fit content.
2. Select Privacy & Security, then Cookies or SSL.
3. Content is pushed down past the bottom and the OK/Cancel/Help buttons are half hidden.

1. Open from MailNews. Prefpane opens to fit content.
2. Select Advanced, then Mouse Wheel.
3. Prefwindow expands to fit content.

Comment 32

[rsx11m]

Interesting - so sometimes the patch works, sometimes it doesn't, but apparently differently for different users. :-\

Comment 33

[[e-mail]]

Attachment: Revised patch

OK, so I was able to reproduce this if I used Windows 7 and visited panes in the correct order (I found that the cookies pane was best for this). I've switched to another method of measuring the height.

Comment 34

[rsx11m]

Comment on attachment 758716 [details] [diff] [review]
Revised patch

Nice, this works fine for me with both Windows 7 default and Windows Classic desktop themes. The preferences window opens at 41em if it fits, or expands to the height as needed. When changing panes, the height now expands to fit it if necessary, and then stays at that height until I close the window.

While this resolves the main issue of content cut off, I'd still think that the change in attachment 757057 [details] [diff] [review] to extend the default height to 43em on Windows is useful. With 41em, it works, but there are quite a few panes where the window has to resize. In contrast, with 43em and otherwise default desktop theme settings, all but the Mouse Wheel pane (which is really long but hidden in Advanced) fit fine without the need to resize.

Comment 35

[rsx11m]

(In reply to rsx11m from comment #34)
> I'd still think that the change in attachment 757057 [details] [diff] [review]
> to extend the default height to 43em on Windows is useful.

(I meant "on top of Neil's patch" not instead of, in case that was ambiguous.)

Comment 36

[Phoenix]

(In reply to [e-mail] from comment #20)
> (I'm curious to know what happens if you have a multiple monitor or remote
> desktop setup and your window moves from an accelerated to an unaccelerated
> graphics card - does the line height change then?)
I tried to test this setup, but failed, even with first Vista drivers back from March 2007 hardware acceleration looks only on primary card drivers and became enabled for all video card (and working, haha), so no chance to drag window from accelerated to not accelerated monitor.

Comment 37

[Ian Neal]

Comment on attachment 757057 [details] [diff] [review]
Option C: Windows only, with instructions

I'm only testing on Fedora, so Neil should have the final say on this.

Comment 38

[Ian Neal]

Comment on attachment 758716 [details] [diff] [review]
Revised patch

r=me

Comment 39

[[e-mail]]

Comment on attachment 758716 [details] [diff] [review]
Revised patch

Pushed comm-central changeset 2eb297b4b15e.

Not sure whether this is a big enough win to take on branches but nothing ventured, nothing gained.

[Approval Request Comment]
Regression caused by (bug #): DirectWrite fonts
User impact if declined: Some preference panes get cut off
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None

Comment 40

[rsx11m]

Comment on attachment 757057 [details] [diff] [review]
Option C: Windows only, with instructions

>-<!ENTITY  prefWindow.size             "width: 52em; height: 41em;">
>+<!ENTITY  prefWindow.size             "width: 52em; height: 43em;">

Neil, do we want this in addition to your patch to avoid frequent resizing of the window with DirectWrite fonts enabled?

Comment 41

[[e-mail]]

http://hg.mozilla.org/releases/comm-aurora/rev/98964551fc10
http://hg.mozilla.org/releases/comm-beta/rev/25097e6756c7

Comment 42

[[e-mail]]

[Hmm, not sure my tracking flag changes were appropriate here]

Comment 43

[rsx11m]

^ It depends how you define "fixed" in this case... ;-)

Comment 44

[Philip Chee]

Comment on attachment 758716 [details] [diff] [review]
Revised patch

Sorry for the delay. Pref window now expands down to fit the contents of the currently selected pref-pane. WFM.

Comment 45

[Stefan [:stefanh]]

Is there anything left to do here (seems fixed when looking at comment #39 and comment #41)?

Comment 46

[rsx11m]

Comment on attachment 757057 [details] [diff] [review]
Option C: Windows only, with instructions

This is still waiting for Neil's ui-review and would increase the window height by 2em for Windows only (no other platform would change), thus avoiding that the dialog height has to be adjusted too frequently when switching panes.

That's complementary to Neil's code and also contains testing instructions to enable the DirectWrite fonts to get the maximum height for Windows testing.

Comment 47

[rsx11m]

(not a big deal if it's no longer needed/wanted, but it would be nice to get to some conclusion on the base height of the preferences window to start from.)

Comment 48

[rsx11m]

SeaMonkey 2.19 has been released, thus clearing the tracking request.

Comment 49

[rsx11m]

(In reply to Philip Chee from comment #31)
> 1. Open from MailNews. Prefpane opens to fit content.
> 2. Select Privacy & Security, then Cookies or SSL.

After bug 886099, now also the Certificates prefpane triggers height adjustments with the 41em default and DirectWrite fonts (bot Windows 7 Default and Classic).

Comment 50

[rsx11m]

Comment on attachment 757057 [details] [diff] [review]
Option C: Windows only, with instructions

Neil's fix has been in the release versions for two cycles now and seems to work, thus let's assume that extending the dialog height on Windows is no longer necessary. In fact, I'd prefer a more general solution to the spacing issue with DirectWrite fonts than just reacting to it in this way, which will have to be done in the relevant Core bugs.

Comment 51

[rsx11m]

Closing per previous comment.