Summary: | paintBorder in RenderObject paint two times the corners | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mario Bensi <mario.bensi> | ||||||||||||
Component: | Layout and Rendering | Assignee: | 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
Mario Bensi
2008-08-24 00:10:35 PDT
Created attachment 22957 [details]
test to draw a border
Created attachment 22958 [details]
fix paintBorder
remove the border width when the paintBorder draw the left and rigth side.
Created attachment 22977 [details]
fix paintBorder and add test before borderPaint
Created attachment 22979 [details]
fix paintBorder, add test and fix coding style
Hyatt, beth? any thoughts? This looks sane to me. 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.
We need to compare with IE and Firefox. 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 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 :)
|