1468671 - Tracking Protection in identity popup is broken for longer localizations

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[Francesco Lodolo [:flod]]

Attachment: italian.png

This is how it currently looks in Italian: the entire information about tracking protection being ON or OFF is missing, and OFF is DISATTIVA in Italian…

Comment 1

[Francesco Lodolo [:flod]]

P.S. Ignore the typo if you can read Italian, it's already been fixed

Comment 2

[Francesco Lodolo [:flod]]

Attachment: italian2.png

One more issue (label for the button is overflowing).

Comment 3

[Johann Hofmann [:johannh]]

Thanks for catching this. I hope just making things wrap will fix it 

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Bug 1468671 - Enable identity popup headlines to span multiple lines.

Review commit: https://reviewboard.mozilla.org/r/251376/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251376/

Comment 5

[Johann Hofmann [:johannh]]

So, making the text wrap unfortunately doesn't really seem to work with the flex model, I can't get the container to grow with its children as I want it to. Hence, I switched to using CSS Grid instead.

Comment 6

[:Paolo Amadini]

Comment on attachment 8985875 [details]
Bug 1468671 - Enable identity popup headlines to span multiple lines.

Maybe I'm missing something, but can't you just use the box model and set flex="1" on the labels and maybe align="center" on the container? Seems to work fine for me, although you may want to double check the desired element alignment.

I tested this patch and it is broken when the permissions header wraps.

Comment 7

[Johann Hofmann [:johannh]]

Let me try that, thanks!

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8985875 [details]
Bug 1468671 - Enable identity popup headlines to span multiple lines.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251376/diff/1-2/

Comment 9

[Johann Hofmann [:johannh]]

So I'm 95% sure I tried something like this earlier, probably missed some detail. It works now. Thank you for the suggestion, Paolo!

Comment 10

[:Paolo Amadini]

Comment on attachment 8985875 [details]
Bug 1468671 - Enable identity popup headlines to span multiple lines.

https://reviewboard.mozilla.org/r/251376/#review258596

::: browser/components/controlcenter/content/panel.inc.xul:60
(Diff revision 2)
>        <!-- Tracking Protection Section -->
>        <hbox id="tracking-protection-container"
>              class="identity-popup-section"
>              when-connection="not-secure secure secure-ev secure-cert-user-overridden file">
>          <vbox id="tracking-protection-content" flex="1">
>            <hbox class="identity-popup-preferences-button-container">

Remove the two instances of the unused "identity-popup-preferences-button-container" class.

Comment 11

[Johann Hofmann [:johannh]]

Comment on attachment 8985875 [details]
Bug 1468671 - Enable identity popup headlines to span multiple lines.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251376/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fd500bb5dd8
Enable identity popup headlines to span multiple lines. r=Paolo

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/6fd500bb5dd8

Comment 14

[Maria Berlinger [:mberlinger], Services QA]

Reproduced this bug on an older Nightly it build 62.0a1 (2017-06-13) using the attachment from comment 0 and comment 2.

I can confirm that this bug is verified fixed, on latest Beta it 62.0b5 (20180702164905) on Mac OS X 10.11.6.