Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jrmuizel, Assigned: mchang)

Tracking

({regression})

unspecified
mozilla44
regression
Points:
---

Firefox Tracking Flags

(firefox43 unaffected, firefox44- fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8667424 [details]
out.html

Test attached
(Assignee)

Updated

3 years ago
Assignee: nobody → mchang
Status: NEW → ASSIGNED
Whiteboard: gfx-noted
(Reporter)

Comment 1

3 years ago
Created attachment 8667457 [details]
Screenshot
(Assignee)

Comment 3

3 years ago
Created attachment 8667923 [details] [diff] [review]
Take into account border radius size when calculating min inset box shadow

From attachment 8667468 [details], which is based off the find search bar, we need to take into account the border radius size to ensure that the min inset box shadow is large enough to actually draw the border radius. 

I tried to come up with a reftest for this, but couldn't really think of another way without using a box-shadow / border radius itself. But there really isn't any difference between having the border radius be exactly the right size for the min box shadow or having the border radius be much longer in terms of producing the artifact.
Attachment #8667923 - Flags: review?(mstange)
(Assignee)

Comment 4

3 years ago
Comment on attachment 8667923 [details] [diff] [review]
Take into account border radius size when calculating min inset box shadow

Review of attachment 8667923 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxBlur.cpp
@@ +952,5 @@
>                   extendBy, minInsetBlur);
>    return minInsetBlur.forget();
>  }
>  
> +

nit: I'll delete this.
(Assignee)

Updated

3 years ago
See Also: → bug 1206629
(Reporter)

Comment 6

3 years ago
Please add a test for this case.
[Tracking Requested - why for this release]: Visual regression
Blocks: 1188075
status-firefox43: --- → unaffected
status-firefox44: --- → affected
tracking-firefox44: --- → ?
Keywords: regression
(In reply to Mason Chang [:mchang] from comment #3)
> I tried to come up with a reftest for this, but couldn't really think of
> another way without using a box-shadow / border radius itself.

It's the middle part that's drawing badly, right? So you could have a test with a border radius and a reference without a border radius (both using box-shadows), and then just compare the middle parts by clipping the outer parts away. You can clip by putting a div around it that has overflow:hidden, and then position things correctly using margins.
Attachment #8667923 - Flags: review?(mstange) → review+
(Assignee)

Comment 10

3 years ago
Created attachment 8668202 [details] [diff] [review]
Part 2: Box shadow reftest with large border radii

Successful try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3d3ff1b8e1d
Attachment #8668202 - Flags: review?(mstange)
Comment on attachment 8668202 [details] [diff] [review]
Part 2: Box shadow reftest with large border radii

Review of attachment 8668202 [details] [diff] [review]:
-----------------------------------------------------------------

Is the bug only reproducible with fractional values?

The leftClip/rightClip elements aren't really clips, they're covers. I hadn't thought of just covering up the edges, but it's a good idea. You should probably rename clip to cover everywhere, and you can remove the overflow:hidden because it doesn't actually do anything.
https://hg.mozilla.org/mozilla-central/rev/b03af020cad5
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
(Assignee)

Comment 13

3 years ago
Created attachment 8668460 [details] [diff] [review]
Part 2: Box shadow reftest with large border radii

Updated with feedback from comment 11.

> Is the bug only reproducible with fractional values?

No, but it's the most glaring difference with the bug. I couldn't get the exact "find search bar" look without it. Instead, I'd get an extra thick border. This test case is from the CSS specified in Firefox's front end in the focused find bar.
Attachment #8668202 - Attachment is obsolete: true
Attachment #8668202 - Flags: review?(mstange)
Attachment #8668460 - Flags: review?(mstange)
Duplicate of this bug: 1210619
Attachment #8668460 - Flags: review?(mstange) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1c9516a8f8f5 - we haven't yet gotten around to running your test on 10.10 opt, but on 10.10 debug, https://treeherder.mozilla.org/logviewer.html#?job_id=15679163&repo=mozilla-inbound

And no, it doesn't run on try unless you specifically ask for 10.10, and yes, that sucks.
(Assignee)

Comment 20

3 years ago
I marked the reftest as fuzzy on OS X 10.10. It's a 1 pixel color difference off.
Given that this is fixed already, I don't see a reason to track this for 44 anymore.
tracking-firefox44: ? → -
You need to log in before you can comment on or make changes to this bug.