Clean up hardcoded 30px height increment for dialogs with titlebars (in the prefs)
Categories
(Toolkit Graveyard :: Notifications and Alerts, defect, P2)
Tracking
(firefox91 fixed)
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: Gijs, Assigned: mhowell)
References
(Blocks 1 open bug)
Details
(Whiteboard: [proton-cleanups] [priority:2c])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
As noted in https://phabricator.services.mozilla.com/D114757#inline-637516, this value is kind of weird. It's been there for a long time but it's not clear what it relates to - the actual titlebar height is known, and I just used this._titleBar
as a canary for whether we were in the prefs or in a browser window in bug 1699430. We should work out what this value corresponds to in the prefs, and at the very least document it, if not change it to rely more directly on the "real" value by measuring something, so that if CSS changes the value doesn't go out of date or something.
Updated•3 years ago
|
Updated•3 years ago
|
Reporter | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
It isn't immediately obvious that the hardcoded 30 pixel value this patch
replaces was meant to represent strictly the height of the dialog's title bar
(including border), but I do believe that is how it was intended, based on
these two things:
- Using 0 instead of 30 when no titlebar is present at least appears to be the
correct behavior, or if nothing else it's a reasonable enough behavior that
it prevents anything looking broken. - 30 pixels is the exact height of the titlebar including border as it
actually appeared on in-content preferences dialog boxes at the time the
hardcoded value was originally introduced here in bug 1128237.
So, based on that, it looks like what's needed here is a value that is zero
when no titlebar exists, or if there is one, then its computed height including
border. As it turns out, we were already computing exactly that value for
another purpose, so this patch simply plugs that in here in place of the
hardcoded constant.
Comment 3•3 years ago
|
||
bugherder |
Updated•1 year ago
|
Description
•