[webkit-reviews] review granted: [Bug 199049] Preview of <picture> element doesn't match element bounds : [Attachment 372597] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jun 20 16:11:35 PDT 2019
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 199049: Preview of <picture> element doesn't match element bounds
https://bugs.webkit.org/show_bug.cgi?id=199049
Attachment 372597: Patch
https://bugs.webkit.org/attachment.cgi?id=372597&action=review
--- Comment #20 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 372597
--> https://bugs.webkit.org/attachment.cgi?id=372597
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=372597&action=review
> Source/WebCore/dom/Range.cpp:1824
> + LayoutRect localBounds = renderer->borderBoundingBox();
auto
> Source/WebCore/dom/Range.cpp:1825
> + Optional<LayoutRect> rootClippedBounds =
renderer->computeVisibleRectInContainer(localBounds, &renderer->view(),
{false, false, visibleRectOptions });
Weird spacing. Auto?
> Source/WebCore/dom/Range.cpp:1826
> +
No blank line
> Source/WebCore/dom/Range.cpp:1829
> + FloatRect snappedBounds =
snapRectToDevicePixels(*rootClippedBounds,
node->document().deviceScaleFactor());
auto
> Source/WebCore/dom/Range.h:121
> + IncludeRectsForDeepElements = 1 << 1,
Would be better as UseVisibleBounds or something I think
> Source/WebCore/testing/Internals.cpp:5103
> +Internals::TextIndicatorData::TextIndicatorData(WebCore::TextIndicatorData
data)
TextIndicatorData is big to pass by value.
> Source/WebCore/testing/Internals.cpp:5108
> +Internals::TextIndicatorData::~TextIndicatorData()
= default
> Source/WebCore/testing/Internals.h:833
> + struct TextIndicatorData {
Name duplication with WebCore::TextIndicatorData is pretty confusing.
> Source/WebCore/testing/Internals.h:837
> + TextIndicatorData(WebCore::TextIndicatorData);
halp
More information about the webkit-reviews
mailing list