Site identity top panel height is not the same as the Connection Security top panel's
Categories
(Firefox :: Site Identity, defect, P3)
Tracking
()
People
(Reporter: itiel_yn8, Assigned: itiel_yn8)
References
(Regression)
Details
(Keywords: regression)
Attachments
(4 files, 2 obsolete files)
See attached.
Changing this value here:
https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/browser/themes/shared/customizableui/panelUI.inc.css#1703
from 4px
to 0.6rem
fixes this for me, but I'm not sure where else that .panel-header
may be in use which this change will not be suitable for.
Comment 2•5 years ago
|
||
Is this a recent regression? Paul and Nihanth have been working on this very recently, so that might have caused it
Comment 3•5 years ago
|
||
In Bug 1574196 the padding of the subview changed, Nihanth added some padding in Bug 1576929. Do we want it to have the same height? In this case we could consider using the built-in panelview header for the main view as well (now that the overflow is fixed).
Comment 4•5 years ago
|
||
I would say that the same height (unless there's overflow) is preferable, yeah.
Comment 5•5 years ago
|
||
We have the same issue in the Protections Panel. I removed the height: 40px
rule because it seemed unnecessary and didn't allow the header to grow for the overflow case, but it appears the mainview header has a min-height: 40px
rule which is why it's bigger.
I think it should be sufficient to apply the min-height: 40px
rule to the subview headers as well. Maybe even apply it to all PanelMultiView view headers.
Comment 6•5 years ago
|
||
Updated•5 years ago
|
Comment 7•5 years ago
|
||
This bug is way more complicated than it looks at first glance, I don't think we can land this in 70.
(In reply to Johann Hofmann [:johannh] from comment #2)
Is this a recent regression? Paul and Nihanth have been working on this very recently, so that might have caused it
This regressed by bug 1559258.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 10•2 years ago
•
|
||
Hi emilio, I was told you could help me out here.
In the patch attached here I'm attempting to unify how the various panel headers are being created and displayed.
After making the needed changes, I got to a state in which the identity panel (when visiting https://permission.site/ for example) and the permissions panel are being cut off from the bottom. The hacky workaround I found to this was to add -moz-box-flex: 1
to them.
Somewhat related, the edit bookmark panel gets a permanent scrollbar no matter what (or how many) items I try to set display: none
on.
All 3 panels get fixed if I either:
- Replace the
min-height
of.panel-header
inpanelUI-shared.css
withheight
(which is not a good fix because I want it to expand when the text inside is lengthy), or - Add
overflow: hidden
to.panel-header
. This has no negative side effect AFAICS.
This raises a few questions though:
- Is the
overflow: hidden
addition the right fix? If so, why? If not, what would you suggest as an alternative? - If I rename the permissions panel header to, say, 8 times of the following:
Permissions for permission.site
(with a whitespace between each repetition) and toggleoverflow: hidden
on and off, I can see a difference in the padding. Not a big deal but annoying. Anything that can be done about this? (ignore this question ifoverflow: hidden
is not the solution)
Thanks!
Assignee | ||
Comment 11•2 years ago
|
||
Comment 12•2 years ago
|
||
(In reply to Itiel from comment #10)
Hi emilio, I was told you could help me out here.
In the patch attached here I'm attempting to unify how the various panel headers are being created and displayed.
After making the needed changes, I got to a state in which the identity panel (when visiting https://permission.site/ for example) and the permissions panel are being cut off from the bottom. The hacky workaround I found to this was to add-moz-box-flex: 1
to them.
Yeah, so... You're mixing flexbox and XUL layout (like the <toolbarbutton>
s etc) very indiscriminately, so I'm not surprised stuff gets a bit confused. In general it'd be great to move towards using HTML only in these panels, is that very hard? I see toolbarbutton
doesn't honor display
, but maybe it should? Or maybe we can turn them into regular html:button
s?
Somewhat related, the edit bookmark panel gets a permanent scrollbar no matter what (or how many) items I try to set
display: none
on.
Which panel is this? The star one, right?
All 3 panels get fixed if I either:
- Replace the
min-height
of.panel-header
inpanelUI-shared.css
withheight
(which is not a good fix because I want it to expand when the text inside is lengthy), or
Yeah, that's not good :/
- Add
overflow: hidden
to.panel-header
. This has no negative side effect AFAICS.
This adds a scroll frame between the flexbox and the xul box, so probably fixes some of the weirdness you're seeing (because XUL / HTML scroll frames are similar and a bit better tested). So if switching more fully to HTML layout is not feasible, and you don't want headers to overflow, this seems like a reasonable path forward.
This raises a few questions though:
- Is the
overflow: hidden
addition the right fix? If so, why? If not, what would you suggest as an alternative?
It's the "right fix" in the sense that it sorta fixes it, because of the above. The real right fix is not mixing XUL and HTML layout modes to the extent possible.
- If I rename the permissions panel header to, say, 8 times of the following:
Permissions for permission.site
(with a whitespace between each repetition) and toggleoverflow: hidden
on and off, I can see a difference in the padding. Not a big deal but annoying. Anything that can be done about this? (ignore this question ifoverflow: hidden
is not the solution)
I didn't dig much into why, but I suspect a similar wrong interaction between XUL and HTML because I can't reproduce on a reduced HTML example...
Updated•2 years ago
|
Assignee | ||
Comment 13•2 years ago
|
||
Updated•2 years ago
|
Comment 14•1 year ago
|
||
Pushed by itiel_yn8@walla.com: https://hg.mozilla.org/integration/autoland/rev/d099556427a2 Share logic behind panel headers across the UI r=willdurand,Gijs,fluent-reviewers,flod
Comment 15•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Assignee | ||
Comment 16•1 year ago
|
||
For the QE folks:
Please make sure that this patch hasn't regressed anything in panels in general, and that in ALL of them (across platforms) the panel header is the same height. Also test compact mode and touch mode.
Thanks!
Comment 17•1 year ago
|
||
I have reproduced this bug on an affected Nightly build 108.0a1 (20221103214316), with Win 10 x64.
The issue is verified as fixed following the instructions from the above comment, no issues were found while checking this on latest Beta 109.0ab9, across platforms: Win 10 x64, macOS 11 and Ubuntu 22.
Description
•