background images spill beyond rounded borders / -moz-border-radius

VERIFIED FIXED in mozilla1.9beta3

Status

()

Core
Layout
P3
normal
VERIFIED FIXED
18 years ago
9 years ago

People

(Reporter: Hixie (not reading bugmail), Assigned: vlad)

Tracking

(4 keywords)

Trunk
mozilla1.9beta3
css-moz, perf, relnote, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

18 years ago
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

18 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

18 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
Last Resolved: 18 years ago
Resolution: --- → REMIND

Comment 3

18 years ago
Verifying REMIND. If anyone has objections, please reopen.
Status: RESOLVED → VERIFIED

Comment 4

17 years ago
*** Bug 55650 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 5

17 years ago
Reopening and moving to Future...
Status: VERIFIED → REOPENED
Keywords: testcase
Resolution: REMIND → ---
Target Milestone: M14 → Future

Comment 6

17 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

17 years ago
Keywords: mozilla1.2
(Reporter)

Comment 7

17 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
Created attachment 63769 [details]
Testcase

Here's another testcase which also shows the background color spilling out even
if there is a broken background image.

Comment 9

16 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

16 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

14 years ago
*** Bug 211094 has been marked as a duplicate of this bug. ***
*** Bug 230755 has been marked as a duplicate of this bug. ***

Comment 13

14 years ago
*** Bug 234823 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Summary: background images spill beyond rounded borders → background images spill beyond rounded borders / -moz-border-radius

Updated

14 years ago
Keywords: mozilla1.2

Updated

13 years ago
Assignee: dcone → nobody
Status: REOPENED → NEW
QA Contact: ian → core.layout

Updated

13 years ago

Comment 14

13 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

13 years ago
*** Bug 276083 has been marked as a duplicate of this bug. ***

Comment 16

13 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

13 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

12 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

12 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.
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

11 years ago
roc says it should be possible to fix this with Cairo.
reproduced in today's trunk on Linux with Cairo
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
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... ;)
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

10 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

10 years ago
Adding "perf" keyword based on rumors that Firefox UI could use fewer DOM elements if this were fixed.
Keywords: perf
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.
Created attachment 279977 [details] [diff] [review]
patch

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)
(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

10 years ago
Blocks: 387345
Flags: blocking1.9?

Comment 34

10 years ago
Adding to blocking list based on Comment 28 as a way to improve perf..
Flags: blocking1.9? → blocking1.9+

Comment 35

10 years ago
Created attachment 290809 [details]
Fixed TestCase

border_radius_spil.html
Created attachment 291099 [details] [diff] [review]
slightly updated patch

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+
Whiteboard: [needs landing]
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/reftests/bugs&command=DIFF_FRAMESET&file=reftest.list&rev1=1.280&rev2=1.281&root=/cvsroot
Flags: in-testsuite+
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
Last Resolved: 18 years ago10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Keywords: relnote

Updated

10 years ago
Whiteboard: [needs landing]
Target Milestone: Future → mozilla1.9 M11

Comment 39

10 years ago
Should there be reftests covering the 1x1 case as well?

Comment 40

10 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
(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

10 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; 
}

Updated

10 years ago
Blocks: 359568

Comment 43

10 years ago
There is still an issue with actual text content inside the box,
please see bug 409547.

Comment 44

10 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.
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

10 years ago
I think it just needed to be added to the "new for FF3" notes.

Comment 47

10 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

10 years ago
Created attachment 303228 [details]
Displays non-clipping of background images

Comment 49

10 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

10 years ago
Created attachment 303232 [details]
Fixed testcase: div with background image to inherit border-radius

So, no need to re-open the background image clipping is now as it should be.
Attachment #303228 - Attachment is obsolete: true
Attachment #303232 - Attachment mime type: text/plain → text/html
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

10 years ago
this bug is not fixed, per comments 40 and 42.

Comment 53

10 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

10 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

10 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

10 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

10 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

10 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

10 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

10 years ago
the very recently checked in bug 359568 solves the <textbox> corner overflow portion of the problem here.

Comment 61

10 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.
Depends on: 468314
You need to log in before you can comment on or make changes to this bug.