115526 - clipping and border drawing need ConditionRect

Status: RESOLVED WORKSFORME

(Core :: Web Painting, defect, P1)

Description

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

Clipping and border drawing need to pass through ConditionRect, at least on
Linux.  See attached testcase, which should show a really tall BODY element with
border, background, and clipping in the same place.  The background is right,
but the border and clipping are wrong.

Other platforms should be tested as well.

Comment 1

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

Attachment: testcase

Comment 2

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

*** Bug 123146 has been marked as a duplicate of this bug. ***

Comment 3

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

The duplicate shows problems with dashed border drawing code as well.

Comment 4

[Boris Zbarsky [:bzbarsky]]

*** Bug 128649 has been marked as a duplicate of this bug. ***

Comment 5

[Boris Zbarsky [:bzbarsky]]

*** Bug 134533 has been marked as a duplicate of this bug. ***

Comment 6

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

Attachment: attempt at a fix (not yet tested)

I haven't tested this yet, and there's also an "XXX" comment about a bug I
already know it has.

Comment 7

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

That's still not quite right, even when I changed the patch like this:

48c48
< @@ -1037,23 +1039,77 @@
---
> @@ -1037,23 +1039,80 @@
105c105,107
< +        // XXX What if we're out-of-bounds on a different side?!
---
> +        // XXX What if we're out-of-bounds on a different side?!  (Note
> +        // also that ConditionPoint might divide by zero in such a
> +        // case.)
116a119
> +      prevConditioned = PR_FALSE;

Comment 8

[Boris Zbarsky [:bzbarsky]]

*** Bug 167936 has been marked as a duplicate of this bug. ***

Comment 9

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

*** Bug 171917 has been marked as a duplicate of this bug. ***

Comment 10

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

Attachment: perhaps a better patch

This seems to work, except for some invalidation problems on borders that may
or may not be in this area of the code.  (They probably are, though.)

Comment 11

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

*** Bug 188675 has been marked as a duplicate of this bug. ***

Comment 12

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

The patch for bug 86249 will probably fix many of the border-related symptoms of
this bug.

Comment 13

[Per Ångström]

The testcase looks worse in Firefox 0.8 and Mozilla 1.6 than in Mozilla 1.5. I
don't know how the clipped part is supposed to look, but in Mozilla 1.5 the
yellow area is nicely contained within the green frame all the way down.

In Firefox 0.8 and Mozilla 1.6 however, the yellow spills over the bottom green
border.

Comment 14

[Per Ångström]

There is still a problem with the yellow overflowing the bottom of the rectangle.

Seen in Mozilla 1.8a3/Linux and Netscape 7.2/Linux.

Comment 15

[Per Ångström]

Comment #14 applies to 1.8a5 as well.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a5) Gecko/20041121

Comment 16

[Per Ångström]

I see no improvement in 1.8a6.

Given that this bug has the highest Priority (P1), has major Severity, and
blocks a number of other bugs, I think it ought to be fixed in Mozilla 1.8.
Thus, I'm nominating this bug to block the 1.8b release.

Comment 17

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

too late for 1.8b, transferring nomination to 1.8b2

Comment 18

[Per Ångström]

For the record: the testcase shows no visible improvement in Mozilla 1.8b.

Comment 19

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

The last five comments have all been of no help whatsoever.  Please stop adding
non-technical comments to this bug.  If you continue, I won't be able to find
the actually useful comments within them and will be less likely to fix the bug.

Comment 20

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

Attachment: updated patch

Comment 21

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

This doesn't seem to fix bug 180877, bug 252241, or bug 290473.

Comment 22

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

We probably only want ConditionRect to be used on GTK1. GTK2 should already be
OK, in theory.

You probably want a typedef for nsAutoBuffer<GdkPoint,8>.

Comment 23

[chris hofmann]

too late for beta2 - try for next milestone

Comment 24

[Mats Palmgren (inactive)]

David, now that gfx/src/gtk/ has been removed on trunk, is this bug/patch
still relevant for trunk?

Comment 25

[Zack Weinberg (:zwol)]

I am not clear on what the correct rendering of the test case ought to have been, but since we no longer support Gtk1, I'm inclined to think this is no longer relevant.  But I'm not going to yank a bug out from under dbaron. :)  It can moulder in the proper component till he gets around to it again.

Comment 26

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

Attachment: testcase

attachment 61907 [details] updated for changes to CSS 'overflow' propagation rules for HTML and BODY.

Comment 27

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

Yeah, I no longer see any problems with the testcase (though I recall we still did have problems in the early cairo days).

Comment 28

[Zack Weinberg (:zwol)]

Do you think it's worth checking in a reftest?