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)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9beta3

People

(Reporter: ian, Assigned: vlad)

References

(
URL
)

Details

(4 keywords)

Attachments

(4 files, 2 obsolete files)

Testcase
(deleted), text/html
Details
Fixed TestCase
(deleted), text/html
Details
slightly updated patch
(deleted), patch
roc
: review+
roc
: superreview+
Details | Diff | Splinter Review
Fixed testcase: div with background image to inherit border-radius
(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.
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
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
Verifying REMIND. If anyone has objections, please reopen.
Status: RESOLVED → VERIFIED
*** Bug 55650 has been marked as a duplicate of this bug. ***
Reopening and moving to Future...
Status: VERIFIED → REOPENED
Keywords: testcase
Resolution: REMIND → ---
Target Milestone: M14 → Future
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...
Keywords: mozilla1.2
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
Attached file Testcase (deleted) — 
Here's another testcase which also shows the background color spilling out even
if there is a broken background image.
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?

I doubt this will get worked on by any of the layout people until after CSS3 has
a border-radius equivalent.
*** Bug 211094 has been marked as a duplicate of this bug. ***
*** Bug 230755 has been marked as a duplicate of this bug. ***
*** Bug 234823 has been marked as a duplicate of this bug. ***
Summary: background images spill beyond rounded borders → background images spill beyond rounded borders / -moz-border-radius
Keywords: mozilla1.2
Assignee: dcone → nobody
Status: REOPENED → NEW
QA Contact: ian → core.layout
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. :))
*** Bug 276083 has been marked as a duplicate of this bug. ***
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.
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);
    }
  }

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.
(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)
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.
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?
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.
Attached patch patch (obsolete) (deleted) — — Splinter Review 
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.
Blocks: 387345
Flags: blocking1.9?
Adding to blocking list based on Comment 28 as a way to improve perf..
Flags: blocking1.9? → blocking1.9+
Attached file Fixed TestCase (deleted) — 
border_radius_spil.html
Attached patch slightly updated patch (deleted) — — Splinter Review 
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)
Whiteboard: [needs landing]
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
Keywords: relnote
Whiteboard: [needs landing]
Target Milestone: Future → mozilla1.9 M11
Should there be reftests covering the 1x1 case as well?
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?

?

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; 
}
Blocks: 359568
There is still an issue with actual text content inside the box,
please see bug 409547.

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?
I think it just needed to be added to the "new for FF3" notes.
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).
Attached file Displays non-clipping of background images (obsolete) (deleted) — 

  

  



    
    

    
  
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.

  

  



    
    

    
  


  
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

    
    

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

  

  



    
    

    
  
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.

  

  



    
    

    
  
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.

  

  



    
    

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

  

  



    
    

    
  
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.

  

  



    
    

    
  
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.

  

  



    
    

    
  
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.

  

  



    
    

    
  
(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'.



  

  



    
    

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

  

  



    
    

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: