Bug 20495

Summary: paintBorder in RenderObject paint two times the corners
Product: WebKit Reporter: Mario Bensi <mario.bensi>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Minor CC: bdakin, hyatt, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 58761    
Bug Blocks:    
Attachments:
Description Flags
test to draw a border
none
fix paintBorder
mario.bensi: review-
fix paintBorder and add test before borderPaint
none
fix paintBorder, add test and fix coding style
hyatt: review-
fix paintBorder in RenderObject and remove the bug on acid2 eric: review-

Description Mario Bensi 2008-08-24 00:10:35 PDT
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 : 

------------------------
|                      |
------------------------
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
|  |                |  |
------------------------
|                      |
------------------------
Comment 1 Mario Bensi 2008-08-24 00:21:15 PDT
Created attachment 22957 [details]
test to draw a border
Comment 2 Mario Bensi 2008-08-24 00:23:50 PDT
Created attachment 22958 [details]
fix paintBorder

remove the border width when the paintBorder draw the left and rigth side.
Comment 3 Mario Bensi 2008-08-25 04:15:23 PDT
Created attachment 22977 [details]
fix paintBorder and add test before borderPaint
Comment 4 Mario Bensi 2008-08-25 05:21:55 PDT
Created attachment 22979 [details]
fix paintBorder, add test and fix coding style
Comment 5 Eric Seidel (no email) 2008-08-27 17:40:13 PDT
Hyatt, beth? any thoughts?  This looks sane to me.
Comment 6 Dave Hyatt 2008-08-27 18:26:32 PDT
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.
Comment 7 Dave Hyatt 2008-08-27 18:26:58 PDT
We need to compare with IE and Firefox.

Comment 8 Mario Bensi 2008-09-01 06:20:58 PDT
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.
Comment 9 Eric Seidel (no email) 2008-09-02 03:14:50 PDT
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 :)
Comment 10 Simon Fraser (smfr) 2011-04-19 11:46:23 PDT
http://trac.webkit.org/changeset/84273