CSS Outline and border-radius Topic is solved
Moderators: trava90, athenian200
- RealityRipple
- Keeps coming back
- Posts: 763
- Joined: 2018-05-17, 02:34
- Location: Los Berros Canyon, California
- Contact:
CSS Outline and border-radius
The spec on outline was altered a bit ago to say it should shape to the border edge, and thus be affected by border-radius. bug #315209 covers the changes, but I wanted to check one thing first. Does the Drive-by cleanup apply to us? It looks like it's no longer expecting the possibility of a document-less entry, getting context from mPresContext->Document() instead. Is this going to differ for us because of our non-document contexts?
Re: CSS Outline and border-radius
I honestly don't know off the top of my head and would have to research if we have document-less contexts to contend with. It does seem to just be to be able to report issues with the border radius though, so I'm not sure it harms anything. The BZ bug (as often the case) doesn't at all give a reason this drive-by-cleanup is safe, anywhere -- and this has caused sec bugs in the past when assumptions were incorrect and "cleanup" happened.
We should probably keep this in place for now. It may be safe to remove, but it may not be, and I'd prefer not to have a crash case because mPresContext is null.
We should probably keep this in place for now. It may be safe to remove, but it may not be, and I'd prefer not to have a crash case because mPresContext is null.
{{This headspace for lease}}
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
- RealityRipple
- Keeps coming back
- Posts: 763
- Joined: 2018-05-17, 02:34
- Location: Los Berros Canyon, California
- Contact:
Re: CSS Outline and border-radius
Sounds good. Also, should we stick with the way FF implemented (with a toggle pref, keeping -moz-outline-radius's code in-tact), should we just do the border-radius changes without the pref, or should we pull out all the outline-radius code in favor of border-radius? I kind of don't think the pref is necessary, but we might wanna keep outline-radius, just for any possible backward compatibility problems.
Edit: I'm gonna tinker with this for a few. Looking through the browser's code, there's a few uses of -moz-outline-radius in the default theme and elsewhere, which likely means other themes also use it, and I'd like to include a bit better fallback than the existing code supports, so those don't break.
OK, in fact, there's some uses of -moz-outline-radius that has a different value from the border-radius in some OS-matching theme code... Soooo, -moz-outline-radius as primary, with border-radius as the fallback if none of the outline-radius corners are non-zero?
Also, I think they double-negatived themselves in the IsInvisibleInRect() function... They returned !HasNonZeroCorner in the new HasRadius function, which they then also !'d. I guess with "Non" in "HasNonZeroCorner" it's optionally a triple negative?
Edit: I'm gonna tinker with this for a few. Looking through the browser's code, there's a few uses of -moz-outline-radius in the default theme and elsewhere, which likely means other themes also use it, and I'd like to include a bit better fallback than the existing code supports, so those don't break.
OK, in fact, there's some uses of -moz-outline-radius that has a different value from the border-radius in some OS-matching theme code... Soooo, -moz-outline-radius as primary, with border-radius as the fallback if none of the outline-radius corners are non-zero?
Also, I think they double-negatived themselves in the IsInvisibleInRect() function... They returned !HasNonZeroCorner in the new HasRadius function, which they then also !'d. I guess with "Non" in "HasNonZeroCorner" it's optionally a triple negative?
Re: CSS Outline and border-radius
Yes. Let the prefixed outline version override the border-radius prop if both are defined.RealityRipple wrote: ↑2024-10-09, 16:47-moz-outline-radius that has a different value from the border-radius in some OS-matching theme code... Soooo, -moz-outline-radius as primary, with border-radius as the fallback if none of the outline-radius corners are non-zero?
Sometimes there's whole stacks of negations in Moz code (drives me nuts...) Carefully check if their logic is correct; maybe double-check with later Firefox code to see if it was changed after the fact in an unreferenced bug?RealityRipple wrote: ↑2024-10-09, 16:47Also, I think they double-negatived themselves in the IsInvisibleInRect() function... They returned !HasNonZeroCorner in the new HasRadius function, which they then also !'d. I guess with "Non" in "HasNonZeroCorner" it's optionally a triple negative?
{{This headspace for lease}}
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
- RealityRipple
- Keeps coming back
- Posts: 763
- Joined: 2018-05-17, 02:34
- Location: Los Berros Canyon, California
- Contact:
Re: CSS Outline and border-radius
Nope, looks like they haven't caught it. https://searchfox.org/mozilla-central/source/layout/painting/nsDisplayList.cpp#4050
FF's original code was:
The change was to:
Their new code reads:
You can kinda see how it happened.
FF's original code was:
Code: Select all
if (borderBox.Contains(aRect) &&
!nsLayoutUtils::HasNonZeroCorner(outline->mOutlineRadius)) {
if (outline->mOutlineOffset._0 >= 0.0f) {
Code: Select all
bool nsDisplayOutline::HasRadius() const {
const auto& radius =
StaticPrefs::layout_css_outline_follows_border_radius_enabled()
? mFrame->StyleBorder()->mBorderRadius
: mFrame->StyleOutline()->mOutlineRadius;
return !nsLayoutUtils::HasNonZeroCorner(radius);
}
...
if (borderBox.Contains(aRect) && !HasRadius() &&
outline->mOutlineOffset.ToCSSPixels() >= 0.0f) {
Code: Select all
bool nsDisplayOutline::HasRadius() const {
const auto& radius = mFrame->StyleBorder()->mBorderRadius;
return !nsLayoutUtils::HasNonZeroCorner(radius);
}
...
if (borderBox.Contains(aRect) && !HasRadius() &&
outline->mOutlineOffset.ToCSSPixels() >= 0.0f) {
Re: CSS Outline and border-radius
Yup, so Firefox would not draw outlines properly then if I understand correctly, omitting them if there are rounded corners and the rect is clipped...
I guess it is a corner case... only happens if the straight edge outlines are clipped but the corners would still be visible because rounded?
I guess it is a corner case... only happens if the straight edge outlines are clipped but the corners would still be visible because rounded?
{{This headspace for lease}}
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
- RealityRipple
- Keeps coming back
- Posts: 763
- Joined: 2018-05-17, 02:34
- Location: Los Berros Canyon, California
- Contact:
Re: CSS Outline and border-radius
Speaking of corner cases, we have one ourselves:
We're treating unset and zero as identical (calling HasNonZeroCorner), but if someone wanted a border-radius of some value (other than zero on at least one corner) but manually includes a -moz-outline-radius that is specifically all zeroes, it will use the border-radius value. I doubt there's any practical implementations of this on any website or theme, but... should we make a "HasNonUnsetCorner" function and use that instead? And if so, what should we count as "fallback to border-radius" besides eStyleUnit_Null? "normal"? "auto"? "none"?
We're treating unset and zero as identical (calling HasNonZeroCorner), but if someone wanted a border-radius of some value (other than zero on at least one corner) but manually includes a -moz-outline-radius that is specifically all zeroes, it will use the border-radius value. I doubt there's any practical implementations of this on any website or theme, but... should we make a "HasNonUnsetCorner" function and use that instead? And if so, what should we count as "fallback to border-radius" besides eStyleUnit_Null? "normal"? "auto"? "none"?
Last edited by RealityRipple on 2024-10-09, 20:10, edited 1 time in total.
Re: CSS Outline and border-radius
I think we can safely ignore that corner case. It makes no logical sense to explicitly set an outline radius to zero (i.e. no radius) while setting a border radius...
But if you want to, we can be explicit for that case and add the extra check method. Just... please find a better name then than NonUnset (double negative before even starting...) in that case maybe "HasSetCorner" or something? if not already in use...
But if you want to, we can be explicit for that case and add the extra check method. Just... please find a better name then than NonUnset (double negative before even starting...) in that case maybe "HasSetCorner" or something? if not already in use...
{{This headspace for lease}}
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite
- RealityRipple
- Keeps coming back
- Posts: 763
- Joined: 2018-05-17, 02:34
- Location: Los Berros Canyon, California
- Contact:
Re: CSS Outline and border-radius
Honestly it's easier if we just don't allow that. If someone really wants to, they can have an outline radius of 0.001px or something.
Oh, but this does make for some clunky transitioning if you try to use a -moz-outline-radius of 0. It's "approach a sharp corner and then suddenly flip to the broder-radius".
Just for best-implementation purposes, I'll check for the unit types of "null" and "auto". That's easy enough. I figure "none" should count as a specific directive not to have an outline radius.
Edit: Not that easy after all... Apparently the outline system is constructed with 0-value coordinate-type corners, so the unit type is always "eStyleUnit_Coord" even if it's entirely unset. There's no good way to implement this the "right" way without rewriting a lot more otherwise-unrelated code.
Oh, but this does make for some clunky transitioning if you try to use a -moz-outline-radius of 0. It's "approach a sharp corner and then suddenly flip to the broder-radius".
Just for best-implementation purposes, I'll check for the unit types of "null" and "auto". That's easy enough. I figure "none" should count as a specific directive not to have an outline radius.
Edit: Not that easy after all... Apparently the outline system is constructed with 0-value coordinate-type corners, so the unit type is always "eStyleUnit_Coord" even if it's entirely unset. There's no good way to implement this the "right" way without rewriting a lot more otherwise-unrelated code.