1540085 - Manage Cookies and Site Data values are offset

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: 2019-03-29 12_02_19-Window.png

Affected versions

• Firefox 66.0.2
• Firefox Beta 67.0b6
• Firefox Nightly 68.0a1

Affected platforms

• Windows 10 (x64)
• macOS 10.12
• Ubuntu 18.04 (x64)

Steps to reproduce

1. Start Firefox
2. Access the following link: https://bug1377104.bmoattachments.org/attachment.cgi?id=8882153
3. Click on "Persist me" and then click on allow
4. Access about:preferences#privacy and click on "Manage Data" from Cookies and Site Data

Expected result

• The values from Cookies, Storage and Last used are retaining their place in the column.

Actual result

• The values are offset by the addition of (Persistent), see the attachment.

Regression range

• First bad: af2f85201ec26b3a639e9e2ba94e805c5f5890a1
• Last good: 45697af1f4e6c0be4e5888b7222877ebae759e84
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2ec04569f9bdc1fe3dc838e4a544a41120f73225&tochange=af2f85201ec26b3a639e9e2ba94e805c5f5890a1
• Potential regressor: Bug 1457021

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

this seems like a layout issue due to the Storage column not being null. I'm not sure what's the way to solve it in the current XBL/CE CSS flex model, but I'm surprised Fluent is affecting this because we didn't really touch layout in that bug.

Comment 2

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

We know that changing to async filling of locale text affected various approaches to sizing so this seems very relevant here. Zibi, can you please investigate this?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1540085 - Use label.value rather than text content in site data settings UI to use cropping. r?jaws

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Ugh! Simple regression. I fixed it by bringing back label.value which I lost last time, but tbh, I'd love us to handle that better in richlistbox handler so that value and textContent of the label are treated equally.

Comment 5

[Francesco Lodolo [:flod]]

Is this something we want to uplift to 67, or it can wait for bug 1536507 (ftl2ftl migration)?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Looks like gandalf is on this, so P1'ing it.

Comment 7

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

I think we should look to uplift this. There will be other opportunities for ftl2ftl migration that we can test with that won't have the regression status.

Comment 8

[Francesco Lodolo [:flod]]

Then the sooner it lands the better, otherwise we risk going into release with a bunch of untranslated strings.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2237e45cd4a
Use label.value rather than text content in site data settings UI to use cropping. r=jaws,flod

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e2237e45cd4a

Comment 11

[Francesco Lodolo [:flod]]

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

Is this something we want to uplift to 67, or it can wait for bug 1536507 (ftl2ftl migration)?

Just realized this was wrong, because the existing message isn't copied as-is, there's a message reference that changes from the old value.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9055009 [details]
Bug 1540085 - Use label.value rather than text content in site data settings UI to use cropping. r?jaws

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1457021
• User impact if declined: Values in the data settings UI can be visually misaligned.
• 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: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a simple change to two strings that has been verified on Nightly.
• String changes made/needed: site-usage-pattern, site-usage-persistant

Comment 14

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #13)

• String changes made/needed: site-usage-pattern, site-usage-persistant

At this point I'm OK with uplifting this.

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9055009 [details]
Bug 1540085 - Use label.value rather than text content in site data settings UI to use cropping. r?jaws

Has l10n-drivers approval from flod, patch is straightforward and fixes a P1 regression, uplift approved for 67 beta 10, thanks.

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

We have STR in comment #0, let's get it verified by QA as well.

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b5b8c302a1a

Comment 18

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced the issue on Windows 10 x64 using the STR from the description with 67.0b8, build ID: 20190404130536.

Confirming the fix across platforms (Win 10 x64, macOS 10.14 and Ubuntu 16.04 x64) on latest Nightly, build ID 20190410215612.

Leaving the qe-verify+ flag in place until this gets verified on Beta, too.

Comment 19

[Cornel Ionce [:noni] [Hubs QA]]

Confirming the fix on Fx 67.0b11 (buildID 20190415085659), too.