470250 - Middle part of border-image doesn't show when all border-widths are 0

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P2)

Description

[Markus Stange [:mstange]]

Attachment: testcase

The attached testcase should show 3x3green-1DD813.png scaled to 50x50 pixels.
But it doesn't.

Comment 1

[Markus Stange [:mstange]]

Attachment: reference

In the reference I added a column of transparent pixels to the left of the image and set the left border width to 1px.

Comment 2

[Zack Weinberg (:zwol)]

Mine.  Will fix as part of the work currently being done in bug 455364.

Comment 3

[Zack Weinberg (:zwol)]

Attachment: partial fix

Here's a fix for the primary symptom.  nsFrame was assuming that a zero-width border on all four sides meant it didn't need to bother drawing the border at all, but if there's a border-image that's not true.

If applied to trunk, the reftest (based on mstange's test case) will fail on two counts: <img> isn't scaled up the same way border-image is, and the upper left hand corner of the <img> is not in the same place as the upper left hand corner of the <div>.  These will both be fixed in bug 455364.  The patches there right now don't quite do it but I'm working on it.

Comment 4

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

I think all three callers of HasBorder want this change, so I think you should just change the semantics of HasBorder instead.

Comment 5

[Zack Weinberg (:zwol)]

Attachment: fix

Yeah, it does look like all callers want that.  Changed.  Also fixed an error in the test case which was causing the "upper left hand corner of the img is not in the same place as the div" problem.

This + all three patches in bug 455364 passes reftests on Linux.

Comment 6

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

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

r+sr=dbaron

Maybe even add a test for the issues with the other callers of HasBorder?  (No need to ask for re-review for that.)

Comment 7

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

Zack, should this have checkin-needed?

Comment 8

[Zack Weinberg (:zwol)]

Not yet; the patches for bug 455364 need to land first.  I'm hoping to get those done by the end of the day.

Comment 9

[Zack Weinberg (:zwol)]

Attachment: fix v2: with tests for the table case, sorta [Checkin: See comment 12 & 13]

Rediffed, and I tried to write tests for the other callers of HasBorder as suggested in comment 6.  I wasn't entirely successful.  The new 470250-2 reftest tries to test the nsTableCellFrame::BuildDisplayList code that calls HasBorder, but gdb says it never actually goes in there.  I have no idea how to write a test for nsFrame::ComputeSimpleTightBounds.

Comment 10

[Zack Weinberg (:zwol)]

tagging for checkin, since it looks like 455364 has stuck.

Comment 11

[Zack Weinberg (:zwol)]

Comment on attachment 356653 [details] [diff] [review]
fix v2: with tests for the table case, sorta
[Checkin: See comment 12 & 13]

blocker, so canceling a191 request.

Comment 12

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/ec1a2f55de07

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 356653 [details] [diff] [review]
fix v2: with tests for the table case, sorta
[Checkin: See comment 12 & 13]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b8573b63ff87

after fixing context for
{
patching file layout/reftests/border-image/reftest.list
Hunk #1 FAILED at 2
1 out of 1 hunks FAILED
}

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b8573b63ff87

The tests are failing:
this should probably not have landed before bug 455364 ! :-<

Comment 15

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

Correct, please back it out

Comment 16

[Serge Gautherie (:sgautherie)]

I preferred to land bug 455364 and bug 477732, after asking Zack.

Comment 17

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090618 Shiretoko/3.5pre ID:20090618042848

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090618 Shiretoko/3.5pre ID:20090618031205

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090618 Minefield/3.6a1pre ID:20090618031143