Closed Bug 448193 Opened 16 years ago Closed 16 years ago

with border inset/outset, 1px, and -moz-border-radius, bottom and right sides are drawn in wrong color

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: Paenglab, Assigned: vlad)

References

()

Details

(Keywords: regression, testcase, verified1.9.1)

Attachments

(4 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008072703 Minefield/3.1a2pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008072703 Minefield/3.1a2pre When the border has a 3D effect style with 1px and -moz-border-radius, no 3D effect is drawn. If the border is more than 1px, it's drawn correctly. Regression window: okay in nightly from 2008-07-23-03-mozilla-central, not okay in nightly from 2008-07-24-03-mozilla-central I assume Bug 424423 has regressed this. Reproducible: Always Steps to Reproduce: 1. See URL
Keywords: regression
Attached file testcase (deleted) —
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [Regression] No 3D effects with border 1px and -moz-border-radius → No 3D effects (inset, outset) with border 1px and -moz-border-radius
Blocks: 424423
Keywords: testcase
Requesting Wanted 1.9.1 as this is a clear regression in layout rendering (border inset radius) (note radius is becoming a web standard, as also webkit/opera/safari/chrome support it, so FF should not regress here...)
Flags: wanted1.9.1?
Requesting Blocking 1.9.1 as this is a clear regression in layout rendering (border inset radius) (note radius is becoming a web standard, as also webkit/opera/safari/chrome support it, so FF should not regress here...) This bug causes content and chrome to render border with 1px inset/outset/ridge/groove combined with radius wrong.
Flags: blocking1.9.1?
Blocks: 458131
Assignee: nobody → vladimir
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Priority: -- → P3
I've looked at this a little. First, the bug only affects inset and outset, not groove and ridge. (Groove and ridge are intentionally degraded to solid at 1px, and have been for some time. There might should be some code to adjust the colors so they're not so blatantly different from what's drawn at width 2px, but that would be a separate bug.) I've adjusted the bug summary. Second, this has nothing to do with the CSS3 (elliptical) border-radius patches, so I've dropped bug 431176 from the blocks list, but it would be good to include this case in the test suite being written in bug 458131, so this still blocks that, and I've added ctalbert to the cc for this one. As for the actual cause of the bug, I think it's a clipping error, probably introduced in the patches for bug 368247 and/or bug 424423. I think this because if you compare the rendering for 'border: 1px outset red' with 'border: 1px solid; border-color: #ff7f7f #b30000 #b30000 #ff7f7f' with nonzero border radius, you will see that the former is ever so slightly thicker; this is especially visible at the corner curves. I do not see the fix immediately; vlad may have more luck.
Blocks: 368247
No longer blocks: border-radius
Summary: No 3D effects (inset, outset) with border 1px and -moz-border-radius → with border inset/outset, 1px, and -moz-border-radius, bottom and right sides are drawn in wrong color
No longer blocks: border-radius
Attached patch fix (deleted) — Splinter Review
Fix. What was actually happening was that the full border was being drawn twice, once with each color for the 3d effect. I don't know how to do a reftest for this; I can create one that will check whether inset is different from solid, but that would pass even without this patch due to the double-painting.
Attachment #342131 - Flags: review?(zweinberg)
(In reply to comment #7) > Fix. What was actually happening was that the full border was being drawn > twice, once with each color for the 3d effect. I thought that must have been what was going on. > I don't know how to do a > reftest for this; I can create one that will check whether inset is different > from solid, but that would pass even without this patch due to the > double-painting. I have one. I'll merge it and confirm.
Attached patch reftest (deleted) — Splinter Review
so here's the reftest I have, which demonstrates that your patch works, Vlad; however, I'm getting a failure due to two lousy pixels. As I currently get about 20 reftest failures due to single-pixel differences (on two different computers, yet) I am gonna post it anyway and ask that you try it.
Attachment #342176 - Flags: review?(vladimir)
Attachment #342131 - Flags: superreview?(dbaron)
Attachment #342131 - Flags: review?(zweinberg)
Attachment #342131 - Flags: review+
Comment on attachment 342131 [details] [diff] [review] fix anyway, the patch works, so. (Procedurally speaking, am I supposed to be able to r+ things, and if so, when?)
(yes, I did read it and check that it *ought* to fix the bug, as well as that it does.)
Attachment #342176 - Flags: review?(vladimir) → review+
Comment on attachment 342176 [details] [diff] [review] reftest Hm, yeah, that reftest should work. I'll add it to my patch when I check in.
I take that back, the reftest fails due to the small pixel differences; I'll check it in as commented out with a comment to that effect
Pushed this out by accident as part of my patch set; it's a simple enough patch. I pushed out the reftest as well, but it's commented out, ideally until we can add some variation in reftests...
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Verified as fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081010 Minefield/3.1b2pre. Thanks!
Status: RESOLVED → VERIFIED
(In reply to comment #14) > Pushed this out by accident as part of my patch set; it's a simple enough > patch. I pushed out the reftest as well, but it's commented out, ideally until > we can add some variation in reftests... FWIW I don't think adding some kind of tolerance threshold to reftests is a good way to go. We'll never be able to find the "right" tolerance algorithm or threshold value. I've had some success at tracking down tiny pixel variations and eliminating them by fixing code or changing the test. Where genuine variation is allowed, you can usually cover up the varying pixels with censoring shapes.
Comment on attachment 342131 [details] [diff] [review] fix sr=dbaron
Attachment #342131 - Flags: superreview?(dbaron) → superreview+
Keywords: verified1.9.1
Keywords: fixed1.9.1
Since we did wind up adding fuzzy(), I added the reftest files (which must have missed an hg add originally) and uncommented the manifest line and made it fuzzy for those two pixels in https://hg.mozilla.org/integration/mozilla-inbound/rev/4f0f5bf7a824.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: