538909 - gradient(s) not painted correctly in border-edge

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[philippe (part-time)]

Attachment: test case

testcase:
box A, basic: a box with a gradient background and a semi-transparent border - rgba() border-color

AR: the left and top border show the background-color instead of the gradient-colors
ER: the gradient is painted/repeated under all the borders.

More comical - box B in the testcase, using this with '-moz-background-origin: content;' and 'background-repeat: repeat-x;'.

(doing the same with an image shows the correct result)

This is the case for both -moz-linear-gradient and -moz-radial-gradient.

Comment 1

[philippe (part-time)]

Attachment: screenshot

Top rendering of test case in WebKit
Bottom: Minefield latest, OS X.

Comment 2

[Markus Stange [:mstange]]

So the real bug is that repeating gradient images aren't repeated left or top of the background origin, right?

Comment 3

[philippe (part-time)]

(In reply to comment #2)
> So the real bug is that repeating gradient images aren't repeated left or top
> of the background origin, right?
Yes.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 421173 [details] [diff] [review]
fix

I presume the old code would have been fine if integer division did round-down rather than round-towards-zero?  (i.e., that's what this is fixing)

>+static nscoord
>+FindTileStart(nscoord aDirtyCoord, nscoord aTilePos, nscoord aTileDim)
>+{
>+  NS_ASSERTION(aTileDim > 0, "Non-positive tile dimension");
>+  nscoord delta = aDirtyCoord - aTilePos;
>+  if (delta >= 0) {
>+    // delta is the offset from the left edge of the tile to aDirtyCoord
>+    return aDirtyCoord - (delta % aTileDim);
>+  }
>+  // Let delta be the offset from aDirtyCoord to the right edge of the tile
>+  delta = aTilePos + aTileDim - aDirtyCoord;
>+  return aDirtyCoord - (delta % aTileDim);

Seems like the "+ aTileDim" doesn't do anything (given that you % by aTileDim), so this line could just be "delta = -delta".  Which means the whole thing could simplify to:
  NS_ASSERTION(aTileDim > 0, "Non-positive tile dimension");
  nscoord delta = PR_ABS(aDirtyCoord - aTilePos);
  return aDirtyCoord - (delta % aTileDim);
But that doesn't actually make sense.

Perhaps this makes more sense:
  nscoord delta = (aDirtyCoord - aTilePos) % aTileDim
  if (delta < 0) {
    delta += aTileDim;
  }
  return aDirtyCoord - delta;
though I'm not sure.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Oops, I shouldn't have chosen a test border-width that's exactly half a tile!

The easy way to do this is using floating point and NS_floor.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 421193 [details] [diff] [review]
fix v2

r=dbaron, although it seems like there should be a clean way that doesn't involve going into floating point

Comment 9

[Mark]

Hi, i also reported this bug (see comment #8). It contains another possible test case.

And is this one going to be fixed for firefox 3.6? I can't find that on top of this page...

Good luck,
Mark

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/945792408b2c

Comment 11

[philippe (part-time)]

(In reply to comment #10)
> http://hg.mozilla.org/mozilla-central/rev/945792408b2c

Does it makes sense to get this on the 1.9.2 branch ?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This fixes a rather serious bug in a new feature in 3.6 (CSS Gradients):  they don't work correctly underneath the top or left border (when the border is dotted/dashed, partially opaque, or transparent), and we should get it in for 3.6.2.

Comment 13

[Brandon Sterne (:bsterne)]

Comment on attachment 421193 [details] [diff] [review]
fix v2

We missed 1.9.2.2.  Moving approval request forward.

Comment 14

[LegNeato]

Comment on attachment 421193 [details] [diff] [review]
fix v2

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we
are still working on 1.9.2.4 on the relbranch

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed on 1.9.2 by transplanting from mozilla-central:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d90854e79525