1502951 - Column text is offset to the left and not aligned with headers when there's a vertical scrollbar

Status: VERIFIED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[:Harald Kirschner :digitarald]

Attachment: image.png

Reported from a Windows 10 user.

Comment 1

[Itiel]

Not sure if related, but this is reproducable on RTL.

Comment 2

[Florian Quèze [:florian]]

(In reply to Itiel from comment #1)
> Not sure if related, but this is reproducable on RTL.

That seems worth investigating, thanks!

Comment 3

[Florian Quèze [:florian]]

(In reply to Itiel from comment #1)
> Not sure if related, but this is reproducable on RTL.

When testing RTL by setting intl.uidirection to 1 in about:config, I see something that looks like the screenshot from bug 1504236, and I'm fixing these issues, but I still don't reproduce this bug.

Comment 4

[Gyula Palko[:gpalko]]

Attachment: resize.PNG

Comment 5

[Gyula Palko[:gpalko]]

I can reproduce this, by resizing the window(making it smaller than the list) or having the list of tasks exceeding the window height. See my attachment.

Comment 6

[Gyula Palko[:gpalko]]

Attachment: resizeWidth.PNG

Also, there's another UI issue, if I resize the window width to be smaller- "Type" overlaps the "Name" field and task names are no more visible. See my second screenshot.

Comment 7

[Florian Quèze [:florian]]

(In reply to Gyula Palko from comment #5)
> I can reproduce this, by resizing the window(making it smaller than the
> list) or having the list of tasks exceeding the window height. See my
> attachment.

Thanks!

Your attachment 9023000 [details] also shows a useless vertical sidebar (I noticed this on Windows and Linux yesterday evening).

Comment 8

[Florian Quèze [:florian]]

Attachment: Patch

- Fix the misalignment between the header text and the body text when there's a vertical scrollbar (this is fixed using position: fixed for the header and making the scrollbar apply to the whole table rather than just the body).
- Avoid the useless vertical scrollbar on Windows. This was due to a problem on the line "height: calc(100vh - 2em - 1px);" that I'm removing. "2em" was actually not enough to account for the size of the header, as the size of the header is max(2em, 2*line-height)... which ended up being 1px more than 2em on Windows.
- Avoid the issue of the Name column being so small that the 'Name' label would overflow into the next column. I fixed this by giving the whole table a minimum width of 40em. The header (which is positioned, so sized independently) also need the min-width rule. When resizing the window to a size smaller than 40em, an horizontal scrollbar was appearing, but made only the table body scroll, as the header has a 'fixed' position. I couldn't find a good fix for that, and such a tiny window is so useless anyway that I decided to just hide the scrollbar in that case (that's the reason for the overflow-x: hidden on the body).

I tested this on Mac and Windows. If you r+ I'll test on Linux too before landing.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3593d60216da
fix about:performance layout issues caused by scrollbars on small windows and on Windows, r=felipe.

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/3593d60216da

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 12

[Florian Quèze [:florian]]

Comment on attachment 9023693 [details] [diff] [review]
Patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: new about:performance

User impact if declined: Broken/misaligned layout. On Windows, a useless vertical scrollbar. This bug also turned out to be the cause of the frustrating bug 1504839.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: Given that this changes several CSS rules, it would be good to verify that the layout is still correct with various themes, and maybe also various font sizes.

List of other uplifts needed: 1503680 (just so that the patches land in the same order on beta)

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Low because CSS only, and brokenness (if any) should hopefully be visible quickly.

String changes made/needed: none

Comment 13

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9023693 [details] [diff] [review]
Patch

css fix for new about:performance, approved for 64.0b10

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7596f6afa475

Comment 15

[Maria Berlinger [:mberlinger], Services QA]

I've managed to reproduce this issue using Firefox 65.0a1 (BuildId 20181101220058) on Windows 10x64.

This issue is verified fixed using Firefox 65.0a1 (BuildId 20181114222927) on Windows 10x64. 

Note: It seems that after resizing the window to its minimum width, the horizontal scrollbar does not appear.
Is this an intended behavior, if not, please let me know and I will file a bug for that issue, separately?

Comment 16

[Florian Quèze [:florian]]

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #15)

> Note: It seems that after resizing the window to its minimum width, the
> horizontal scrollbar does not appear.
> Is this an intended behavior, if not, please let me know and I will file a
> bug for that issue, separately?

This is an expected side effect of the patch, yes. If I didn't hide the horizontal scrollbar, the body of the table was scrolling horizontally but not its header, which was worse than not scrolling at all.

Comment 17

[Maria Berlinger [:mberlinger], Services QA]

This issue is verified fixed using Firefox 64.0b10 on Windows 10x64.