Bug 121616 - [CSS Shapes] Support block content with inline content around floats in shape-inside
Summary: [CSS Shapes] Support block content with inline content around floats in shape...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zoltan Horvath
URL:
Keywords:
Depends on: 102846
Blocks: 121615
  Show dependency tree / graph
 
Reported: 2013-09-19 09:56 PDT by Zoltan Horvath
Modified: 2013-10-08 10:30 PDT (History)
5 users (show)

See Also:

Attachments
proposed patch (30.66 KB, patch)
2013-10-01 16:57 PDT, Zoltan Horvath 		no flags Details | Formatted Diff | Diff
proposed patch (30.66 KB, patch)
2013-10-01 17:56 PDT, Zoltan Horvath 		no flags Details | Formatted Diff | Diff
proposed patch (30.65 KB, patch)
2013-10-01 17:58 PDT, Zoltan Horvath 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 2013-09-19 09:56:57 PDT 
Support block content with inline content around floats in shape-inside, look at bug #102846.
Comment 1 Zoltan Horvath 2013-10-01 16:57:33 PDT 
Created attachment 213132 [details]
proposed patch
Comment 2 Zoltan Horvath 2013-10-01 17:56:59 PDT 
Created attachment 213138 [details]
proposed patch
Comment 3 Zoltan Horvath 2013-10-01 17:58:12 PDT 
Created attachment 213139 [details]
proposed patch
Comment 4 Dave Hyatt 2013-10-03 10:47:03 PDT 
Comment on attachment 213139 [details]
proposed patch

r=me
Comment 5 WebKit Commit Bot 2013-10-03 11:23:37 PDT 
Comment on attachment 213139 [details]
proposed patch

Clearing flags on attachment: 213139

Committed r156846: <http://trac.webkit.org/changeset/156846>
Comment 6 WebKit Commit Bot 2013-10-03 11:23:38 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Bem Jones-Bey 2013-10-08 09:54:55 PDT 
Comment on attachment 213139 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=213139&action=review

> Source/WebCore/rendering/RenderBlock.cpp:3371
> +        LayoutSize floatLogicalSize = LayoutSize(childBox->logicalWidth(), childBox->logicalHeight());

I just came across this in the code, and it looks completely wrong to me. The floatingObject's dimensions are those of the margin box, and the logicalWidth and Height of the childBox are those of the content box. Floats are supposed to be positioned based on their margins, so this looks like it wouldn't work properly in the case of a float with margins. Am I missing something?
Comment 8 Bem Jones-Bey 2013-10-08 10:30:18 PDT 
(In reply to comment #7)
> (From update of attachment 213139 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=213139&action=review
> 
> > Source/WebCore/rendering/RenderBlock.cpp:3371
> > +        LayoutSize floatLogicalSize = LayoutSize(childBox->logicalWidth(), childBox->logicalHeight());
> 
> I just came across this in the code, and it looks completely wrong to me. The floatingObject's dimensions are those of the margin box, and the logicalWidth and Height of the childBox are those of the content box. Floats are supposed to be positioned based on their margins, so this looks like it wouldn't work properly in the case of a float with margins. Am I missing something?

By content box, I mean border box, but same point. :-)