[webkit-reviews] review canceled: [Bug 109775] Convert media controls from DeprecatedFlexibleBox to FlexibleBox : [Attachment 188247] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 14 16:45:09 PST 2013


Christian Biesinger <cbiesinger at chromium.org> has canceled Christian Biesinger
<cbiesinger at chromium.org>'s request for review:
Bug 109775: Convert media controls from DeprecatedFlexibleBox to FlexibleBox
https://bugs.webkit.org/show_bug.cgi?id=109775

Attachment 188247: Patch
https://bugs.webkit.org/attachment.cgi?id=188247&action=review

------- Additional Comments from Christian Biesinger <cbiesinger at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=188247&action=review


>> LayoutTests/ChangeLog:8
>> +	    * media/media-document-audio-repaint-expected.txt:
> 
> In cases like this where there's a long list of files you can just delete the
list of files, leaving only the ones that have extra explanatory comments.
> 
> Also, it'd be good to have a description of what changed in these expected
results. Off the top of my head:
> -Names of objects in the render tree dump change.
> -Some widths of items are slightly different due to new flexbox and old
flexbox shrinking over-constrained flex-items using a slightly different
algorithm. Old flexbox shrinks all flex-items evenly. New flexbox shrinks them
in proportion to the size of the flex item.
> -LayoutTests/platform/chromium-linux/media/video-zoom-controls-expected.png
looks strange. It has no play button in the new result. Both the old and new
behavior look wrong though. Do you know what's going on there? It at least
deserves an explanation in the ChangeLog.

Turns out this can be fixed using justify-content: flex-start;. I'll make that
change (and the other changes you requested)


More information about the webkit-reviews mailing list