Bug 20495 – paintBorder in RenderObject paint two times the corners

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Mario Bensi]

here a log of drawBorder coordinate in paintBorder :

x1 : 215 y1 : 474 x2 : 768 y2 : 478
x1 : 215 y1 : 533 x2 : 768 y2 : 537
x1 : 215 y1 : 474 x2 : 219 y2 : 537
x1 : 764 y1 : 474 x2 : 768 y2 : 537

the result is that : 

------------------------
|  |                |  |
------------------------
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
------------------------
|  |                |  |
------------------------

I think we should have that : 

------------------------
|                      |
------------------------
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
------------------------
|                      |
------------------------

[Mario Bensi]

Created attachment 22957 [details]
test to draw a border

[Mario Bensi]

Created attachment 22958 [details]
fix paintBorder

remove the border width when the paintBorder draw the left and rigth side.

[Mario Bensi]

Created attachment 22977 [details]
fix paintBorder and add test before borderPaint

[Mario Bensi]

Created attachment 22979 [details]
fix paintBorder, add test and fix coding style

[Eric Seidel (no email)]

Hyatt, beth? any thoughts?  This looks sane to me.

[Dave Hyatt]

Comment on attachment 22979 [details]
fix paintBorder, add test and fix coding style

I think this needs test cases.  You are in effect choosing one border style to "win" at the corner.  We need to test this to see how we match up.

This is very important for tests like Acid 2.

[Dave Hyatt]

We need to compare with IE and Firefox.

[Mario Bensi]

Created attachment 23099 [details]
fix paintBorder in RenderObject and remove the bug on acid2

I use the old code when the rect is not available and this fix the problem on acid2 and on the text border you have not the border paint two time.

[Eric Seidel (no email)]

Comment on attachment 23099 [details]
fix paintBorder in RenderObject and remove the bug on acid2

I can't evaluate the correctness of this patch, but I can say that the copy-paste code used here is a bad idea.

The if should be used to store the necessary variables, and then a single drawBorder call should be used.

It appears that you're changing where it's drawing from/to.  It seems the code right above this adjusts y and y2 in an if, why can't this change also do the same? (use an if to adjust y and y2?  or copy y and y2 into some other nicely named variables and ajust them there).  Copy/paste of code which is confusing to begin with is a bad idea.  Ideally this method would be cleaned up to use FloatPoint and possibly to take fewer parameters :)

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/84273