Bug 117162 - [CSS Shapes][CSS Exclusions] Split CSS Exclusions and CSS Shapes code
Summary: [CSS Shapes][CSS Exclusions] Split CSS Exclusions and CSS Shapes code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bear Travis
URL:
Keywords:
Depends on:
Blocks: 89256 117173
  Show dependency treegraph
 
Reported: 2013-06-03 12:55 PDT by Bear Travis
Modified: 2013-06-10 14:24 PDT (History)
21 users (show)

See Also:


Attachments
Initial Patch (321.53 KB, patch)
2013-06-03 15:38 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updated patch (278.90 KB, patch)
2013-06-05 17:34 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Adding directory (279.12 KB, patch)
2013-06-06 11:19 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Adding ShapeOutsideInfo.h to sources (279.25 KB, patch)
2013-06-06 14:20 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Adding folders (280.39 KB, patch)
2013-06-06 15:48 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updating patch (280.41 KB, patch)
2013-06-06 16:10 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
cmakelists additions (283.90 KB, patch)
2013-06-07 11:13 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updated, incorporating feedback (283.91 KB, patch)
2013-06-10 11:51 PDT, Bear Travis
achicu: review+
Details | Formatted Diff | Diff
Incorporating feedback (284.00 KB, patch)
2013-06-10 13:38 PDT, Bear Travis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bear Travis 2013-06-03 12:55:07 PDT
With CSS Exclusions & Shapes splitting into two separate features, some code cleanup is required.
http://dev.w3.org/csswg/css-exclusions/
http://dev.w3.org/csswg/css-shapes/

This work will clean up the implementation side.
A subsequent patch will clean up the test side.
Comment 1 Bear Travis 2013-06-03 15:38:04 PDT
Created attachment 203633 [details]
Initial Patch
Comment 2 Bear Travis 2013-06-05 17:34:44 PDT
Created attachment 203894 [details]
Updated patch
Comment 3 Early Warning System Bot 2013-06-05 17:47:52 PDT
Comment on attachment 203894 [details]
Updated patch

Attachment 203894 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/756256
Comment 4 Early Warning System Bot 2013-06-05 17:49:48 PDT
Comment on attachment 203894 [details]
Updated patch

Attachment 203894 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/786051
Comment 5 EFL EWS Bot 2013-06-05 17:54:13 PDT
Comment on attachment 203894 [details]
Updated patch

Attachment 203894 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/709903
Comment 6 EFL EWS Bot 2013-06-05 18:24:02 PDT
Comment on attachment 203894 [details]
Updated patch

Attachment 203894 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/786052
Comment 7 kov's GTK+ EWS bot 2013-06-05 18:35:30 PDT
Comment on attachment 203894 [details]
Updated patch

Attachment 203894 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/758687
Comment 8 Bear Travis 2013-06-06 11:19:04 PDT
Created attachment 203953 [details]
Adding directory
Comment 9 Early Warning System Bot 2013-06-06 11:26:57 PDT
Comment on attachment 203953 [details]
Adding directory

Attachment 203953 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/697563
Comment 10 Early Warning System Bot 2013-06-06 11:27:06 PDT
Comment on attachment 203953 [details]
Adding directory

Attachment 203953 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/790018
Comment 11 EFL EWS Bot 2013-06-06 11:36:59 PDT
Comment on attachment 203953 [details]
Adding directory

Attachment 203953 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/695650
Comment 12 EFL EWS Bot 2013-06-06 11:38:27 PDT
Comment on attachment 203953 [details]
Adding directory

Attachment 203953 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/658714
Comment 13 kov's GTK+ EWS bot 2013-06-06 12:23:10 PDT
Comment on attachment 203953 [details]
Adding directory

Attachment 203953 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/777228
Comment 14 Bear Travis 2013-06-06 14:20:10 PDT
Created attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources
Comment 15 Early Warning System Bot 2013-06-06 14:27:36 PDT
Comment on attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources

Attachment 203967 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/790055
Comment 16 Early Warning System Bot 2013-06-06 14:28:29 PDT
Comment on attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources

Attachment 203967 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/792010
Comment 17 EFL EWS Bot 2013-06-06 14:31:13 PDT
Comment on attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources

Attachment 203967 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/772478
Comment 18 EFL EWS Bot 2013-06-06 14:31:56 PDT
Comment on attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources

Attachment 203967 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/772479
Comment 19 kov's GTK+ EWS bot 2013-06-06 15:44:18 PDT
Comment on attachment 203967 [details]
Adding ShapeOutsideInfo.h to sources

Attachment 203967 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/792025
Comment 20 Bear Travis 2013-06-06 15:48:14 PDT
Created attachment 203976 [details]
Adding folders
Comment 21 Bear Travis 2013-06-06 16:10:19 PDT
Created attachment 203977 [details]
Updating patch
Comment 22 EFL EWS Bot 2013-06-06 16:29:04 PDT
Comment on attachment 203977 [details]
Updating patch

Attachment 203977 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/774331
Comment 23 EFL EWS Bot 2013-06-06 16:33:16 PDT
Comment on attachment 203977 [details]
Updating patch

Attachment 203977 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/790080
Comment 24 Bear Travis 2013-06-07 11:13:10 PDT
Created attachment 204056 [details]
cmakelists additions
Comment 25 Hans Muller 2013-06-07 14:47:13 PDT
It would be helpful if there were a summary of the before/after classnames at the top of the ChangeLog.
Comment 26 Zoltan Horvath 2013-06-07 14:53:54 PDT
Comment on attachment 204056 [details]
cmakelists additions

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

It looks okay to me. 1 comment:

> Source/WebCore/rendering/RenderBlock.cpp:1447
> +    ShapeInsideInfo* shapeInsideInfo;

nit: you can leave this in the block below as it was before.
Comment 27 Bear Travis 2013-06-10 11:51:28 PDT
Created attachment 204176 [details]
Updated, incorporating feedback
Comment 28 Alexandru Chiculita 2013-06-10 13:20:55 PDT
Comment on attachment 204176 [details]
Updated, incorporating feedback

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

r=me

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:118
> +        ShapeInsideInfo* shapeInsideInfo = m_block->layoutShapeInsideInfo();
> +        if (shapeInsideInfo)

nit: The variable could live inside the condition statement. That's more like the WebKit style.

> Tools/ChangeLog:8
> +        Adding the rendering/shapes directory

nit: It looks like you forgot to finish the sentence in the WebKit project Changelogs. "Adding the webcore/rendering/shapes directory to the include list."
Comment 29 Bear Travis 2013-06-10 13:38:11 PDT
Created attachment 204184 [details]
Incorporating feedback
Comment 30 WebKit Commit Bot 2013-06-10 14:23:55 PDT
Comment on attachment 204184 [details]
Incorporating feedback

Clearing flags on attachment: 204184

Committed r151402: <http://trac.webkit.org/changeset/151402>
Comment 31 WebKit Commit Bot 2013-06-10 14:24:00 PDT
All reviewed patches have been landed.  Closing bug.