[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