Bug 24467 - [chromium] default border for IMG is drawn outset instead of inset
Summary: [chromium] default border for IMG is drawn outset instead of inset
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Peter Kasting
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-09 13:22 PDT by Darin Fisher (:fishd, Google)
Modified: 2009-03-13 14:26 PDT (History)
1 user (show)

See Also:


Attachments
patch v1 (5.32 KB, patch)
2009-03-13 13:50 PDT, Peter Kasting
no flags Details | Formatted Diff | Diff
patch v1.1 (5.73 KB, patch)
2009-03-13 14:07 PDT, Peter Kasting
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2009-03-09 13:22:42 PDT
[chromium] default border for IMG is drawn outset instead of inset

GraphicsContext::drawRect is passed the right rectangle, but instead of drawing a rect within those coordinates, we draw it outside of those coordinates.

this causes some layout tests to fail, and it also causes some rendering glitches related to selection.  if a selection runs up to the edge of an IMG, then the selection obscures part of the IMG border, and when the selection is removed, the border is not redrawn properly (presumably because webcore didn't think the IMG was part of the selection).

this layout test is impacted:
LayoutTests/fast/block/float/nested-clearance.html
Comment 1 Peter Kasting 2009-03-13 12:16:43 PDT
The CG drawRect() forcefully draws an inset border using four rects instead of actually using the stroke.  Hyatt hates this but suggests that for now we change to match and put a FIXME in the cross-platform header warning other implementers until this can be fixed.
Comment 2 Eric Seidel (no email) 2009-03-13 12:35:01 PDT
We could also change the name of drawRect() to be self-documenting. :)
Comment 3 Peter Kasting 2009-03-13 13:50:00 PDT
Created attachment 28595 [details]
patch v1

This makes drawRect() ignore the stroke width (like CG does), and adds a warning comment about that to the appropriate header.

It also eliminates some hacky code in Skia's stroke preparation, which seems inappropriate because (a) it didn't match CG's behavior and (b) it would do something obviously wrong when other transforms (e.g. global zoom) were applied.
Comment 4 Peter Kasting 2009-03-13 14:07:50 PDT
Created attachment 28598 [details]
patch v1.1

Updated ChangeLog to add more comments per eseidel.
Comment 5 Eric Seidel (no email) 2009-03-13 14:22:11 PDT
Comment on attachment 28598 [details]
patch v1.1

Looks fine.  Please update the changelog to talk a bit more about the "hacky" code you removed when landing. :)
Comment 6 Peter Kasting 2009-03-13 14:26:34 PDT
Fixed in r41683.