Closed Bug 1521209 Opened 6 years ago Closed 6 years ago

about:performance content hidden behind header with localized builds.

Categories

(Toolkit :: Performance Monitoring, defect)

65 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 - verified
firefox66 + verified
firefox67 + verified

People

(Reporter: david.mozillabug, Assigned: florian)

References

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Open about:performance page.

FTR, I did it using latest nightly release (66.0a1 from 2019-01-18 on Debian sid).

Actual results:

The first element in the table is partially hidden behind the table header.
Zooming in or out or changing window size has no effect.

Expected results:

No overlap :-)

Manually removing from the page CSS the above parameter fixes the issue:
#dispatch-thead { position: fixed; }
To avoid getting empty space, at least one of the above should also be disabled (both being fine):
#dispatch-tbody { display: block; }
#dispatch-tbody { margin-top: 2em; }

I do not see any overlapping in my current windows and Linux (ubuntu) nightly in about:performance.
Could you please attach a screenshot ?

Component: Untriaged → Performance Monitoring
Product: Firefox → Toolkit

Here you are for a screenshot.

In case it can help, I am using a French locale (fr_FR).

It seems that width limit for “Energetic impact” (or something like that) column results in two lines for the header. If I remove the width from the CSS, it also result in no more overlap.

Summary: about:performance content hidden behind header → about:performance content hidden behind header with localized builds.

I think you are right that the line wrap of "Energetic impact" is causing this.
confirming based on the screenshot.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 66 Branch → 65 Branch

this got reported from french, russian and greek locales so far.

Tarek, can you help find an owner to work on this? I'd like to see us fix it in 67 and 66 beta since many users are reporting it.

Flags: needinfo?(tarek)

If it just ends up being a simple CSS fix or something, I'd consider it for a 65 dot release ride-along too.

also need-infoing florian, since he worked on most off this stuff...

Flags: needinfo?(florian)

This was already reported at bug 1511637. I don't have an easy solution, so I guess if we need a quick fix, we'll just have to increase a bit the width of that column.

Flags: needinfo?(tarek)

(In reply to Florian Quèze [:florian] from comment #12)

This was already reported at bug 1511637. I don't have an easy solution, so I guess if we need a quick fix, we'll just have to increase a bit the width of that column.

In the regression triage meeting today we discussed how due to the prominence of the new about:performance in 65, if possible, a quick fix would be ideal for a 65.0.1 ride-along with a better long-term fix post-65.0.x.

Flags: needinfo?(tarek)

wfm

Flags: needinfo?(tarek)

I tried a French localized nightly on OS X, Ubuntu and Windows 10 and couldn't reproduce. The Russian string is even longer than the French one, and I could reproduce the bug when using that string (putting that string in using the devtools inspector). Bug 1524038 says that 11em as a column width is good to fix it for Russian. I put a width of 12em in the patch to be safe.

I looked at https://transvision.mozfr.org/string/?entity=toolkit/toolkit/about/aboutPerformance.ftl:column-energy-impact&repo=gecko_strings to see if we have even longer strings that wouldn't fit within 12em. The only one that doesn't fit is the string for the gd locale: "Buaidh air caitheamh dealain". This would need 14em to fit... but 14em means a lot of empty space when 8em is enough for most locales. We should probably ask if the translation can be changed to a shorter wording. ni?flod for this.

The other CSS change ensures that if we have a column with a header that can't fit, it won't wrap and the overflow will just be hidden.

Flags: needinfo?(florian) → needinfo?(francesco.lodolo)

(In reply to Florian Quèze [:florian] from comment #16)

We should probably ask if the translation can be changed to a shorter wording. ni?flod for this.

I really believe we should figure out how to make the layout not break for long translations. Asking to shorten translation, especially on desktop where space shouldn't be an issue, is a process that doesn't scale.

I understand you want a contained fix for uplift, and that's reasonable. But that shouldn't be the long term solution.

Would it be an option to have 2 lines of text available in the thead by default?

Flags: needinfo?(francesco.lodolo)
Attached image 2 rows header

thank you for the stop gap fix - since this may get included into a 65.0.1 release per comment #8, could you please request uplift for beta and release? (it would be nice if it can be included in 66.0b6 still that's built tomorrow to have people try it out before the fix is ending up on release)

(In reply to Francesco Lodolo [:flod] from comment #17)

Would it be an option to have 2 lines of text available in the thead by default?

I think that would be ugly when all the labels fit on a single line. But that's really more a question for UX (Bryan Bell cc'ed).

Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c679fa99e878 Give more space for the 'Energy Impact' column header, and ensure it never wraps, r=Felipe.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9041897 [details]
Bug 1521209 - Give more space for the 'Energy Impact' column header, and ensure it never wraps, r=felipe.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

about:performance

User impact if declined

The first line in about:performance may be invisible in some locales with some font settings.

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

Use Firefox in a locale where the "Energy Impact" label is translated by a longer string.

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Minimal CSS change

String changes made/needed

none

Attachment #9041897 - Flags: approval-mozilla-release?
Attachment #9041897 - Flags: approval-mozilla-beta?
Assignee: nobody → florian

Comment on attachment 9041897 [details]
Bug 1521209 - Give more space for the 'Energy Impact' column header, and ensure it never wraps, r=felipe.

[Triage Comment]
Adds a bit more room to the Energy Impact column to avoid wrapping in some locales. Approved for 66.0b6.

Attachment #9041897 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-66b-p2]
Flags: qe-verify+

Comment on attachment 9041897 [details]
Bug 1521209 - Give more space for the 'Energy Impact' column header, and ensure it never wraps, r=felipe.

[Triage Comment]
Simple fix for cut-off content in about:performance for some locales. Approved for 65.0.1.

Attachment #9041897 - Flags: approval-mozilla-release? → approval-mozilla-release+
Whiteboard: [qa-66b-p2] → [qa-66b-p2][qa-triaged]

I have managed to reproduce the issue using Fx 66.0a1 with a RU and EL (greek) locale. The issue was not occurring for the FR (french) locale on that same build.

The issue is verified fixed using Fx 67.0a1, Fx66.0b6 and Fx65.0.1(downloaded from taskcluster) for the RU, EL and FR locale. The about:performance header no longer overlaps the first line.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Whiteboard: [qa-66b-p2][qa-triaged]
Whiteboard: [qa-66b-p2][qa-triaged]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: