Closed Bug 1589015 Opened 5 years ago Closed 5 years ago

[Windows] The bottom of the Privacy Panel is too short when there are element that occupy two rows

Categories

(Firefox :: Protections UI, defect, P1)

All
Windows
defect

Tracking

()

RESOLVED FIXED
Firefox 72
Tracking Status
firefox70 + wontfix
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: obotisan, Assigned: nhnt11)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Affected versions

  • Firefox 70.0
  • Nightly 71.0a1

Affected platforms

  • Windows 10 x64

Steps to reproduce

  1. Download a “de” build from https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central-l10n/
  2. Install it and launch the browser with a clean profile.
  3. Load https://www.welt.de/ in a new tab.
  4. Wait until the site is fully loaded and click on the shield icon.
  5. Refresh the page if the issue is not reproducing.

Expected result

  • The bottom of the Privacy Panel has the correct dimensions.

Actual result

  • The bottom of the Privacy Panel is cut off.

Regression range

  • This is not a regression. I can reproduce the issue on Nightly from 2019-09-16 when the Privacy Panel was fully translated.

Additional notes

  • If you restart the browser, the dorhanger has the normal proportions for a few seconds and then the bottom of it is cut off again.
  • In the attached gif you can see both the actual result (welt.de) and expected behaviour (youtube).

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

This is because of the cookies category item's label being dynamically set. descriptionHeightWorkaround is calculating the height for a string that doesn't wrap, and then assumes that it doesn't need to re-calculate it. The solution is to invalidate the height every time we change the label. Patching coming up.

The situation is more complicated than I thought. The reason the height isn't invalidated, is because the toolbarbutton reports its textContent as the concatenation of the two labels it contains, regardless of whether one of them is hidden. This means that the textContent looks constant if we switch between them, so descriptionHeightWorkaround thinks it doesn't need to do anything.

Attachment #9101849 - Attachment description: Bug 1589015 - Invalidate the height of the cookies category item after updating the label. r=johannh → Bug 1589015 - Stop using two label elements in the cookies category item. r=johannh
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Priority: -- → P1
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66b01a30ed12
Stop using two label elements in the cookies category item. r=johannh

We confirmed that the patch seems to be effective - but it wasn't ready to land yet because I hadn't run tests.

Try push with updated patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ecbc121ef53e0da739ca85c830154dacf865aa8

Flags: needinfo?(jhofmann)
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fa914a108591
Stop using two label elements in the cookies category item. r=johannh
https://hg.mozilla.org/integration/autoland/rev/9fbd56a4ec97
Update cookies category label tests. r=johannh
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Nihanth, this is marked as P1 and 71 affected, is that something we could uplift safely to beta or can we let it ride the trains to 72? Thanks

Flags: needinfo?(nhnt11)

Comment on attachment 9101849 [details]
Bug 1589015 - Stop using two label elements in the cookies category item. r=johannh

Beta/Release Uplift Approval Request

  • User impact if declined: In some cases the protections panel is cropped at the bottom, which looks glitchy.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): I'm marking this medium because it's not just a couple of lines of CSS, but it's not very risky either.
  • String changes made/needed:
Flags: needinfo?(nhnt11)
Attachment #9101849 - Flags: approval-mozilla-beta?
Attachment #9102855 - Flags: approval-mozilla-beta?

(In reply to Pascal Chevrel:pascalc from comment #11)

Nihanth, this is marked as P1 and 71 affected, is that something we could uplift safely to beta or can we let it ride the trains to 72? Thanks

I think we can uplift, requested above :)

Comment on attachment 9101849 [details]
Bug 1589015 - Stop using two label elements in the cookies category item. r=johannh

P1, low risk visual fix that baked on nightly for more than a week, uplift approved for 71 beta 6, thanks.

Attachment #9101849 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9102855 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I still managed to reproduce the issue on latest Nightly 72.0a1 on Windows 10 x64. After I closed the browser and reopened it, the doorhanger had the expected dimensions. But when I closed it again and reopened it, the issue reproduced. I think it might be intermittent.

Flags: needinfo?(nhnt11)

Oana, if you can still reproduce this, can you please file a bug blocking this one? I'm not sure anyone has cycles to look further into the edge cases here, but we should track the issue if it exists.

Flags: needinfo?(nhnt11)
You need to log in before you can comment on or make changes to this bug.