Closed
Bug 24998
Opened 25 years ago
Closed 17 years ago
background images spill beyond rounded borders / -moz-border-radius
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
VERIFIED
FIXED
mozilla1.9beta3
People
(Reporter: ian, Assigned: vlad)
References
()
Details
(4 keywords)
Attachments
(4 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details |
This broke out of bug 23704. Background images are not clipped to the rounded
border. See the first part of this test case:
http://www.bath.ac.uk/%7Epy8ieh/m/border-radius-image.html
[assigning straight to dcone, to avoid the pierre|troy -> rods -> dcone route
these bugs normally take...]
The following comments were by german on bug 23704.
------- Additional Comments From german@netscape.com 2000-01-25 10:33 -------
In case somebody wonders we will need this to complete the transition for the
default skin from using fake bitmaps for radii towards using 'real'
stylesheets. Desirable time frame for a fix would be m14, in order to get it
into a potntial beta candidate.
Reporter | ||
Comment 1•25 years ago
|
||
Giving QA to chrisd as with bug 23704.
Severity: major → normal
Keywords: css-moz
QA Contact: petersen → chrisd
Summary: rounded background images spill → background images spill beyond rounded borders
Target Milestone: M14
Comment 2•25 years ago
|
||
Talked with RickG, this will not be supported. The biggest problem is Linux
does not support regions for clipping.
For UI, can GIF's with transparency work here?
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → REMIND
Comment 3•25 years ago
|
||
Verifying REMIND. If anyone has objections, please reopen.
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 5•24 years ago
|
||
Reopening and moving to Future...
Comment 6•24 years ago
|
||
For a simple XUL testcase of this, see bug 55650.
BTW, it turns out that also background color spills out of the rounded border if
any (even non-existent) background-image is assigned, and this is one of my
biggest problems with skin work right now...
Would be nice to get this fixed after focus of NS people is off RTM (perhaps
target mozilla1.0 then). I hope there'll be some possibility to deal with that
Linux problem...
Reporter | ||
Updated•24 years ago
|
Keywords: mozilla1.2
Reporter | ||
Comment 7•24 years ago
|
||
Netscape's standard compliance QA team reorganised itself once again, so taking
remaining non-tables style bugs. Sorry about the spam. I tried to get this done
directly at the database level, but apparently that is "not easy because of the
shadow db", "plus it screws up the audit trail", so no can do...
QA Contact: chrisd → ian
Comment 8•23 years ago
|
||
Here's another testcase which also shows the background color spilling out even
if there is a broken background image.
Comment 9•23 years ago
|
||
This has my vote - this is affecting me, as I want to put a background image on
tab's but cant because of this bug. Anyone have a status?
Reporter | ||
Comment 10•23 years ago
|
||
I doubt this will get worked on by any of the layout people until after CSS3 has
a border-radius equivalent.
Comment 11•21 years ago
|
||
*** Bug 211094 has been marked as a duplicate of this bug. ***
Comment 12•21 years ago
|
||
*** Bug 230755 has been marked as a duplicate of this bug. ***
Comment 13•21 years ago
|
||
*** Bug 234823 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
Summary: background images spill beyond rounded borders → background images spill beyond rounded borders / -moz-border-radius
Updated•21 years ago
|
Keywords: mozilla1.2
Updated•20 years ago
|
Assignee: dcone → nobody
Status: REOPENED → NEW
QA Contact: ian → core.layout
Updated•20 years ago
|
Comment 14•20 years ago
|
||
Bump, comment #10 indicated that this would not be worked on until CSS3 had a
border-radius property. It has since (at least) 2002-11-07, so I guess work can
be started on fixing this bug, right?
http://www.w3.org/TR/2002/WD-css3-border-20021107/#the-border-radius
I'd like to see some headway made (I'd like to do it myself, but there are a
couple factors limiting this: time and ability. :))
Comment 15•20 years ago
|
||
*** Bug 276083 has been marked as a duplicate of this bug. ***
Comment 16•20 years ago
|
||
There are some differences between border-radius (the W3C proposal) and
-moz-border-radius, which may cause backwards-compatibility conflicts.
I am willing to put time into matching the W3C proposal (which I expect will
become a recommendation).
Should I bother, or is the potential incompatibility unwelcome at this time?
If it's welcome, then I'd like to discuss some ideas on how to implement it.
Comment 17•20 years ago
|
||
a solution to the spilling might be to tile the background images as normal,
then remove the curves from the corners by painting them in a transparent colour.
Here is some code to do the painting. It paints in a solid colour, as I can't
seem to get alpha-values working. Place this code at the end of
PaintBackgroundWithSC() in nsCSSRendering.cpp :
static nsPoint polypath[MAXPOLYPATHSIZE];
PRInt32 curIndex,c1Index;
nsFloatPoint aPoints[MAXPATHSIZE];
PRInt16 borderRadii[4];
nsStyleCoord bordStyleRadius[4];
QBCurve cr1,cr2;
QBCurve UL;
UL.MidPointDivide(&cr1,&cr2);
aBorder.mBorderRadius.GetTop(bordStyleRadius[NS_SIDE_TOP]); // topleft
aBorder.mBorderRadius.GetRight(bordStyleRadius[NS_SIDE_RIGHT]); // topright
aBorder.mBorderRadius.GetBottom(bordStyleRadius[NS_SIDE_BOTTOM]); // bottomright
aBorder.mBorderRadius.GetLeft(bordStyleRadius[NS_SIDE_LEFT]); // bottomleft
PRUint8 side = 0;
for (; side < 4; ++side) {
borderRadii[side] = 0;
switch (bordStyleRadius[side].GetUnit()) {
case eStyleUnit_Percent:
borderRadii[side] = nscoord(bordStyleRadius[side].GetPercentValue() *
bgClipArea.width);
break;
case eStyleUnit_Coord:
borderRadii[side] = bordStyleRadius[side].GetCoordValue();
break;
default:
break;
}
}
aRenderingContext.SetColor(NS_RGBA(255,255,255,127));
for (side=0; side < 4; ++side) {
if(borderRadii[side]>0){
switch (side) {
case 0:
aPoints[0].x=borderRadii[0];
aPoints[0].y=0;
aPoints[1].x=cr2.mCon.x;
aPoints[1].y=cr2.mCon.y;
aPoints[2].x=0;
aPoints[2].y=borderRadii[0];
aPoints[3].x=0;
aPoints[3].y=0;
break;
case 1:
aPoints[0].x=aBorderArea.width-borderRadii[1];
aPoints[0].y=0;
aPoints[1].x=aBorderArea.width-cr2.mCon.x;
aPoints[1].y=cr2.mCon.y;
aPoints[2].x=aBorderArea.width;
aPoints[2].y=borderRadii[1];
aPoints[3].x=aBorderArea.width;
aPoints[3].y=0;
break;
case 2:
aPoints[0].x=aBorderArea.width-borderRadii[2];
aPoints[0].y=aBorderArea.height;
aPoints[1].x=aBorderArea.width-cr2.mCon.x;
aPoints[1].y=aBorderArea.height-cr2.mCon.y;
aPoints[2].x=aBorderArea.width;
aPoints[2].y=aBorderArea.height-borderRadii[2];
aPoints[3].x=aBorderArea.width;
aPoints[3].y=aBorderArea.height;
break;
case 3:
aPoints[0].x=borderRadii[3];
aPoints[0].y=aBorderArea.height;
aPoints[1].x=cr2.mCon.x;
aPoints[1].y=aBorderArea.height-cr2.mCon.y;
aPoints[2].x=0;
aPoints[2].y=aBorderArea.height-borderRadii[3];
aPoints[3].x=0;
aPoints[3].y=aBorderArea.height;
}
polypath[0].x = NSToCoordRound(aPoints[0].x);
polypath[0].y = NSToCoordRound(aPoints[0].y);
curIndex = 1;
GetPath(aPoints,polypath,&curIndex,eOutside,c1Index);
polypath[curIndex].x = NSToCoordRound(aPoints[3].x);
polypath[curIndex].y = NSToCoordRound(aPoints[3].y);
curIndex++;
polypath[curIndex].x = NSToCoordRound(aPoints[0].x);
polypath[curIndex].y = NSToCoordRound(aPoints[0].y);
curIndex++;
aRenderingContext.FillPolygon(polypath,curIndex);
}
}
Comment 18•19 years ago
|
||
This also applies when you try to apply -moz-border-radius to a normal <img>
element (with no background-image set). The border goes underneath the image in
this case though, as opposed to above the image as in the earlier testcase.
Comment 19•19 years ago
|
||
(In reply to comment #18)
> This also applies when you try to apply -moz-border-radius to a normal <img>
> element (with no background-image set). The border goes underneath the image in
> this case though, as opposed to above the image as in the earlier testcase.
That seems correct, border-radius isn't supposed to clip the content or anything.
It would be nice to get this fixed though.
Comment 20•19 years ago
|
||
May be a different bug, but text isn't fit into boxes with rounded borders correctly either. In that case it shouldn't be clipped though. It should just fit, ideally with different line widths depending on the box width at that point.
Test Page: http://public.wesleyjames.fastmail.fm/test.html
(I won't upload it, as I think this is the wrong bug, but can't find a more suitable one, and don't want to create a possible dupe yet)
Comment 21•18 years ago
|
||
roc says it should be possible to fix this with Cairo.
Comment 22•17 years ago
|
||
reproduced in today's trunk on Linux with Cairo
Assignee | ||
Comment 23•17 years ago
|
||
Yeah, the fix for this is fairly easy, it's just been low on my list because it's not a regression. Assigning to me, if I ever get under my pile of blockers...
Assignee: nobody → vladimir
Comment 24•17 years ago
|
||
for the sake of human history:
<vlad> gandalf: so the cheap and easy fix is to just set a clip before we draw the image
<vlad> and you can set the clip by calling some of the new methods to calculate the outer radius
<vlad> e.g. see what the rounded border background solid color thing does
<vlad> to create the path
<vlad> and then jsut do that and call Clip() instead of fill
<vlad> and then do the image drawing
<vlad> that should basically do it
<vlad> gandalf: the long-term fix is whack PaintBackground to draw stuff using thebes directly.. I started doing that a while ago but had to abandon it
<vlad> gandalf: not in a bug
<vlad> don't worry about that, I think doing the clip thing should get that particular thing working
<vlad> and should be good enough for 1.9
I'll try the cheap and easy way... ;)
Assignee | ||
Comment 25•17 years ago
|
||
Just noticed -- in the testcase, on the second chunk with the background image, there's an invalidation issue -- if you highlight text in the upper left or upper right corners and then unhighlight, the text/background doesn't get repainted correctly.
Comment 26•17 years ago
|
||
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a8pre) Gecko/2007082004 Minefield/3.0a8pre
I'm not seeing that particular issue on Mac. OS-specific?
Comment 27•17 years ago
|
||
Adding "perf" keyword based on rumors that Firefox UI could use fewer DOM elements if this were fixed.
Keywords: perf
Assignee | ||
Comment 28•17 years ago
|
||
Tabs, in particular, could be significantly changed (to a single box, even, where now there are 4 or more) using a combination of this and bug 378217 (border-image)... I think either one would be sufficient, border-image might be more efficient (faster to draw images), but using true borders would let us scale the UI and have it look nice.
Assignee | ||
Comment 29•17 years ago
|
||
Ok, I took a bit to whip up this patch; it was even simpler than I thought. This seems to work, at least for the few testcases that I tried. There's some unnecessary code duplication, and as I said before, PaintBackground really needs to be completely redone, but that's not going to happen for 1.9.
Attachment #279977 -
Flags: review?(roc)
+ nsRefPtr<gfxContext> ctx = (gfxContext*)
static_cast
+ ctx->Rectangle(RectToGfxRect(dirtyRect, appUnitsPerPixel));
The old code used to snap with PR_TRUE here (in SetClipRect), now you're not spanning. I think you should, the performance hit of non-pixel-aligned cliprects is horrific.
+ borderRadii[side] = nscoord(bordStyleRadius[side].GetPercentValue() *
+ aForFrame->GetSize().width);
Why use the width for all four sides?
Maybe this is the wrong time to ask, but is there a reason this code can't just call aForFrame->GetUsedBorder? Is aBorder possibly not the frame's real border?
Comment on attachment 279977 [details] [diff] [review]
patch
Cancelling request pending comments being addressed
Attachment #279977 -
Flags: review?(roc)
Assignee | ||
Comment 32•17 years ago
|
||
(In reply to comment #30)
> + nsRefPtr<gfxContext> ctx = (gfxContext*)
>
> static_cast
The previous uses of this in the file did c-style casts; I can replace them all with static_cast<>
> + ctx->Rectangle(RectToGfxRect(dirtyRect, appUnitsPerPixel));
>
> The old code used to snap with PR_TRUE here (in SetClipRect), now you're not
> spanning. I think you should, the performance hit of non-pixel-aligned
> cliprects is horrific.
Will do.
> + borderRadii[side] = nscoord(bordStyleRadius[side].GetPercentValue() *
> + aForFrame->GetSize().width);
>
> Why use the width for all four sides?
>
> Maybe this is the wrong time to ask, but is there a reason this code can't just
> call aForFrame->GetUsedBorder? Is aBorder possibly not the frame's real border?
This is what the border code has done in the past (and is done elsewhere) -- percentages are always relative to the width of the element (since as far as I understand it, CSS layout only deals with widths as input). GetUsedBorder returns the border sizes themselves, right? Not sure where it'd be used here
oops, you're right.
Updated•17 years ago
|
Flags: blocking1.9?
Comment 34•17 years ago
|
||
Adding to blocking list based on Comment 28 as a way to improve perf..
Flags: blocking1.9? → blocking1.9+
Comment 35•17 years ago
|
||
border_radius_spil.html
Assignee | ||
Comment 36•17 years ago
|
||
Does snap for clipping to dirtyRect; I didn't do the static_cast change because I want to get rid of NATIVE_THEBES_CONTEXT entirely and just add a aRenderingContext.ThebesContext() call everywhere.
Attachment #279977 -
Attachment is obsolete: true
Attachment #291099 -
Flags: review?(roc)
Attachment #291099 -
Flags: superreview+
Attachment #291099 -
Flags: review?(roc)
Attachment #291099 -
Flags: review+
Updated•17 years ago
|
Whiteboard: [needs landing]
Comment 37•17 years ago
|
||
Flags: in-testsuite+
Assignee | ||
Comment 38•17 years ago
|
||
Checked in; changed Ryan's reftests slightly to be closer to what I had in my tree (mainly, need to use a 2x2 image to avoid some 1x1 optimizations that could affect the result).
Status: NEW → RESOLVED
Closed: 25 years ago → 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Whiteboard: [needs landing]
Target Milestone: Future → mozilla1.9 M11
Comment 39•17 years ago
|
||
Should there be reftests covering the 1x1 case as well?
Comment 40•17 years ago
|
||
if this is intended to fix chrome also, it doesn't seem to. overflow is still there in urlbar using:
textbox, filefield {
-moz-appearance: none !important;
-moz-border-radius: 14px !important;
padding-right: 8px !important;
padding-left: 4px !important;
}
in addition <filefield> doesn't work at all.
the content testpage in #35 does work.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007121905 Minefield/3.0b3pre ID:2007121905
Assignee | ||
Comment 41•17 years ago
|
||
(In reply to comment #40)
> if this is intended to fix chrome also, it doesn't seem to. overflow is still
> there in urlbar using:
I'm not sure what you mean here; can you provide a full testcase, or at least screenshots of what's happening and what you'd expect?
Comment 42•17 years ago
|
||
?
simply put the css, with the xul namespace, in your userchrome.css file, and restart (i use the stylish extension to easier apply changes). the chrome elements' background overflows at the corners; a <filefield> is a type of <textbox> and is found, for one, in Options -> Main (Downloads groupbox - Save files box) - its border disappears completely with moz-border-radius.
i would expect this background overflow fix to also apply to chrome elements borders. is this fix intended to apply to chrome element borders?
here's the entire userchrome.css for completeness:
@namespace url(http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul);
menulist, textbox, filefield {
-moz-appearance: none !important;
-moz-border-radius: 14px !important;
padding-right: 8px !important;
padding-left: 4px !important;
}
Comment 43•17 years ago
|
||
There is still an issue with actual text content inside the box,
please see bug 409547.
Comment 44•17 years ago
|
||
There is also still an issue in combination with -moz-border-*-colors: see bug 414545. The strange thing is that with background-image it works correctly, but not with just background-image.
Comment 45•17 years ago
|
||
Someone suggested that this is something that should be mentioned in docs, but reading quickly over the comments here and glancing at the patch, I don't see anything obvious that stands out as needing to be documented per se. Anyone want to pitch in what I should say if anything in the moz-border-radius doc?
Comment 46•17 years ago
|
||
I think it just needed to be added to the "new for FF3" notes.
Comment 47•17 years ago
|
||
This bug is not fixed and needs to be reopened. I have provided an extension to the test case at http://web.nickshanks.com/browsers/mozilla/border-radius-image which demonstrates this (and attached it to the bug).
Comment 48•17 years ago
|
||
Comment 49•17 years ago
|
||
This test case is not valid.
Rounding of 'background-image' only happens if on the *same* element, the border-radius is specified.
The simple thing to do to get this effect is to replace:
div.bkgnd div div {width: 7em; height: 7em; background: url('http://www.hixie.ch/resources/images/sample.gif'); }
with:
div.bkgnd div div { -moz-border-radius:inherit; width: 7em; height: 7em; background: url('http://www.hixie.ch/resources/images/sample.gif'); }
So that the div with the background image has the same radius as its parent.
Comment 50•17 years ago
|
||
So, no need to re-open the background image clipping is now as it should be.
Attachment #303228 -
Attachment is obsolete: true
Updated•17 years ago
|
Attachment #303232 -
Attachment mime type: text/plain → text/html
Comment 51•17 years ago
|
||
Indeed. I can verify that it is fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021104 Minefield/3.0b4pre ID:2008021104 and the latest test case from Alfred. Background images are cropped correctly.
Status: RESOLVED → VERIFIED
Comment 52•17 years ago
|
||
this bug is not fixed, per comments 40 and 42.
Comment 53•17 years ago
|
||
Background image/color rounding is ONLY applied when that same element has -moz-border-radius set. The elements that 'fail' to round in comment #40 and #42 are child elements which don't have border-radius set.
You have to explicitly set the -moz-border-radius on those elements as well, like so:
@namespace url(http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul);
menulist, textbox, filefield {
-moz-appearance: none !important;
-moz-border-radius: 14px !important;
padding-right: 6px !important;
padding-left: 2px !important;
}
#identity-box,.searchbar-engine-button{
-moz-border-radius: 12px !important;
}
You will also need to this for the other child elements in textbox which apply background color and/or image.
Comment 54•17 years ago
|
||
thanks alfred. yes, it's clear children need this explicitly applied in a complex box like urlbar. but i'm talking about the background-color or background-image, which are properties of <textbox> and not children. these overflow and they should not. i mean, it works that way for content, no?
look at #search-box, the simplest <textbox>, in sidebar history eg. how would you fix that? and <filefield> just doesn't work - there is no border at all, that's simply broken afaict.
Comment 55•17 years ago
|
||
Alta88, I suggest you take this discussion to another bug (or forum).
The issues you are facing are not because of the rendering, but the way the widgets are constructed and styled (which makes it not always easy to change something simple as the border).
To fix <filefield> do the following:
filefield{
border:2px inset threedface!important;
background-image:url("file:///c:/tmp/agif/1041385237.gif")!important;
color:white!important;
}
.fileFieldContentBox{background:transparent none!important}
Note, this even works with animated gifs, rounding and all...
Comment 56•17 years ago
|
||
sorry, your suggestions are improper hacks and certainly not expected behavior. i suggest you reread the title of the bug. and to be forced to explicitly redefine an element's border because it is removed is simply ludicrous. the issues i'm facing are because this 'fix' doesn't work for chrome, period.
perhaps it should be a separate bug, but the patch author hasn't responded whether it was intended for chrome despite being asked, therefore this remains the proper place. not wanting to reopen a bug doesn't actually fix it, btw.
Comment 57•17 years ago
|
||
The original bug was reported on normal plain content rendering where the background-image did not follow the radius rounding of a border (-moz-border-radius).
Whether filefield, textbox and other widgets work correctly or not is a separate subject. I would suggest to create new bugs for those widgets.
The patch did not remove borders, or change anything else.
The only thing it did was to make sure that for an element the background-image is rounded according to the -moz-border-radius set on the same element.
Whether child elements also inherit the radius is yet again another topic, and possibly worth a new bug as well.
Comment 58•17 years ago
|
||
border-radius is not an inherited property (per spec), so if you open a new bug on making -moz-border-radius inherit it would be marked INVALID. If the issue is widgets that are made up of a combination of boxes, you can open a bug on making the inner elements specify -moz-border-radius: inherit in the ua style sheet.
Comment 59•17 years ago
|
||
(In reply to comment #57)
> The original bug was reported on normal plain content rendering where the
> background-image did not follow the radius rounding of a border
> (-moz-border-radius).
>
> Whether filefield, textbox and other widgets work correctly or not is a
> separate subject. I would suggest to create new bugs for those widgets.
>
per comments 4 and 6, the xul version of this bug was duped back to this bug. so really, this bug is not fixed and should be reopened.
> The patch did not remove borders, or change anything else.
> The only thing it did was to make sure that for an element the background-image
> is rounded according to the -moz-border-radius set on the same element.
i don't see how that's true. adding -moz-border-radius to <filefield> causes the border to disappear.
please stop apologizing for this 'fix'.
Comment 60•17 years ago
|
||
the very recently checked in bug 359568 solves the <textbox> corner overflow portion of the problem here.
Comment 61•17 years ago
|
||
> i don't see how that's true. adding -moz-border-radius to <filefield> causes
> the border to disappear.
No, the line
-moz-appearance: none !important;
makes it disappear. In the default theme the border of filefied (and other widgets) are drawn by the 'system appearance', using -moz-appearance: textfield.
By setting -moz-appearance: none one tells to Mozilla/Firefox to not do that, but that the border will be styled using normal css. As that widget doesn't have borders set, it won't therefor draw these.
Regarding comments #4 and #6, and thus bug 55650: This is really solved because if now wants to apply a startrek image to an element and apply -moz-border-radius it will work.
However, one has to take that child elements also have border-radius (see comment #55), that child elements or have no background fill (image or color), or that the also do rounding. See comment #58 why radius is not inherited by default (but one can always do -moz-border-radius: inherit..
please stop discussing this on this bug.
please create a new bug to address your issues.
You need to log in
before you can comment on or make changes to this bug.
Description
•