image overflows the rounded border when using -moz-border-radius

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
Layout: Images
--
minor
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Pavol Rusnak, Assigned: dbaron)

Tracking

(Blocks: 1 bug)

unspecified
mozilla2.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Opera/9.64 (X11; Linux x86_64; U; en) Presto/2.1.1
Build Identifier: 

The behaviour and the workaround is described here:
http://stick.gk2.sk/blog/2009/03/image-with-rounded-corners-using-css3/

I'm attaching the testcase. Also I'm not sure if the behaviour is a bug or a feature according to CSS 3 specification.

Reproducible: Always

Steps to Reproduce:
1. unpack attachment
2 [review]. open index.html
3. compare two images
(Reporter)

Comment 1

9 years ago
Created attachment 369650 [details]
testscase
(Reporter)

Comment 2

9 years ago
issue also reported here:
https://bugs.kde.org/show_bug.cgi?id=188242
https://bugs.webkit.org/show_bug.cgi?id=24880
(Assignee)

Updated

9 years ago
Duplicate of this bug: 497354
(Assignee)

Comment 4

9 years ago
So I think this is actually only supposed to work according to the spec if the image has 'overflow:hidden'.  I'm not sure if we'd do it given that, though.

http://dev.w3.org/csswg/css3-background/#the-border-radius does say:
  # It is recommended that the UA style sheet apply overflow: hidden to
  # elements (such as the <img> element in HTML) that are expected to be
  # replaced elements so that their corners automatically trim to the
  # border radius. 
but I'm pretty scared of the effects that would have on performance (since overflow:hidden implies scrollability), and I'm not particularly happy with the idea of the spec saying that.
Hardware: x86_64 → All
Summary: image overflows the border when using border-radius → image overflows the rounded border when using -moz-border-radius
(Assignee)

Comment 5

9 years ago
Created attachment 382504 [details]
standalone examples
I agree, we don't really want images to become scrollable, it would be a significant perf hit to wrap them all in scrollframes.
This is not the sort of thing that's worth taking a major perf hit for, but I wonder if it's really that bad.

Independent of whether the default behavior of images changes, is there an objection to having explicit overflow!=visible clip to the curve (as modified by the special license for visible scrollbars)?  That's bug 459144.

How much will compositor reduce the overhead of wrapping a scrollframe around an element?

<img> -- or more precisely nsImageFrame -- is special: as far as I can tell, it never overflows (in the absence of border-radius) and therefore the programmatic scrolling that overflow:hidden allows never actually does anything.  Could we perhaps make nsImageFrame do its own overflow:hidden clipping and provide a stub version of the programmatic scrolling interface, then?
(In reply to comment #7)
> Independent of whether the default behavior of images changes, is there an
> objection to having explicit overflow!=visible clip to the curve (as modified
> by the special license for visible scrollbars)?  That's bug 459144.

That sounds OK to me.

> How much will compositor reduce the overhead of wrapping a scrollframe around
> an element?

Not much. We already don't create a native widget for overflow:hidden content.

> <img> -- or more precisely nsImageFrame -- is special: as far as I can tell, it
> never overflows (in the absence of border-radius) and therefore the
> programmatic scrolling that overflow:hidden allows never actually does
> anything.  Could we perhaps make nsImageFrame do its own overflow:hidden
> clipping and provide a stub version of the programmatic scrolling interface,
> then?

We don't need to provide a stub implementation of anything, the default behaviour of scroll* properties for non-scrollable elements should be fine. Perhaps we can just special-case nsImageFrame and certain other frame types so that overflow:hidden doesn't create scrollframes for them.
I'm going to confirm this, because I'm experiencing it, too, and it's kinda a pain.

Also, I don't know if any of this speculation has actually been tested, but this doesn't just happen when the image itself is styled with rounded borders. If you have an image inside of a <p>, for example, and the <p> is the one styled with curved borders and hidden overflow, the image still shows through.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Created attachment 392679 [details]
Expanded testcase

It may be that the <p> has to be styled to be the same size as the image, but I've expanded the original testcase to try to demonstrate both problems in the same file.
Attachment #369650 - Attachment is obsolete: true
Attachment #382504 - Attachment is obsolete: true
(Assignee)

Comment 11

8 years ago
According to http://lists.w3.org/Archives/Public/www-style/2009Aug/0589.html this has been changed to a special case for replaced elements.

So we should probably just go ahead and fix this explicitly for nsImageFrame, nsHTMLCanvasFrame, nsHTMLVideoElement, and probably nsObjectFrame.  (I'm not sure whether we should touch nsFrameFrame or HTML form controls.)

Updated

8 years ago
Blocks: 431176
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/6a0b99dc03a9/clip-to-border-radius has work in progress (works, but no tests yet, and haven't tested anything other than images) for replaced elements.

(We also need to do the clipping for overflow-not-visible; that's an entirely separate patch.)
(Assignee)

Updated

8 years ago
Blocks: 537433
(Assignee)

Updated

8 years ago
Blocks: 459144
(Assignee)

Comment 13

8 years ago
(In reply to comment #12)
> (We also need to do the clipping for overflow-not-visible; that's an entirely
> separate patch.)

...which should go in bug 459144.

Updated

8 years ago
Duplicate of this bug: 578592

Comment 15

8 years ago
Hello, any news on this issue? Sorry if “bumping” a bug is against Bugzilla etiquette. I don't have much to add, except that I regret this could not be improved in Firefox4 (unless feature freeze has not been reached yet?), and that web designers wanting to design with CSS3—an increasing trend—might not think this issue is "minor". ;)

Updated

7 years ago
Blocks: 451134
(Assignee)

Updated

7 years ago
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
blocking2.0: --- → beta6+
Target Milestone: --- → mozilla2.0b6
(Assignee)

Comment 16

7 years ago
Created attachment 472878 [details] [diff] [review]
patch

This applies on top of the patches in bug 459144, where the bulk of the work for this bug is.
Attachment #472878 - Flags: review?(roc)
+nsIFrame::DisplayReplacedContentInBorderRadius(nsDisplayListBuilder* aBuilder,

I think existing conventions would favour a name starting with BuildDisplayList ... say BuildDisplayListForReplacedContent?

But another way of doing this would be to have the replaced elements just create an nsDisplayList containing the replaced-content, and then have a method (possibly in nsLayoutUtils) WrapReplacedContentForBorderRadius(nsIFrame* aFrame, nsDisplayList* aList). I think that would be a bit cleaner.
(Assignee)

Comment 18

7 years ago
Created attachment 472943 [details] [diff] [review]
part 1: change signature of DisplaySelectionOverlay
Attachment #472943 - Flags: review?(roc)
(Assignee)

Comment 19

7 years ago
Created attachment 472944 [details] [diff] [review]
part 2: main patch
Attachment #472878 - Attachment is obsolete: true
Attachment #472944 - Flags: review?(roc)
Attachment #472878 - Flags: review?(roc)
Attachment #472943 - Flags: review?(roc) → review+
Attachment #472944 - Flags: review?(roc) → review+
(Assignee)

Comment 20

7 years ago
http://hg.mozilla.org/mozilla-central/rev/2d01121770f7
http://hg.mozilla.org/mozilla-central/rev/0aa1272e932f
http://hg.mozilla.org/mozilla-central/rev/da15f373d499
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Depends on: 596889
Depends on: 599368
You need to log in before you can comment on or make changes to this bug.