Bug 170913 – Plumb all four obscured insets to WebCore, instead of just top/left

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Plumb all four obscured insets to WebCore, instead of just top/left

[Tim Horton]

Created attachment 307293 [details]
Patch

[Tim Horton]

Maybe this should use FloatBoxExtent?

[Simon Fraser (smfr)]

Comment on attachment 307293 [details]
Patch

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

> Source/WebCore/platform/graphics/FloatRectInsets.h:32
> +class FloatRectInsets {

Can this be a BoxExtent<float>?

[Build Bot]

Attachment 307293 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

lol, I beat you by 10 seconds.

[Wenson Hsieh]

Comment on attachment 307293 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=170913

Maybe add a "(Work towards )<rdar://problem/________>" here?

> Source/WebKit2/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=170913

Maybe add a "(Work towards )<rdar://problem/________>" here?

> Source/WebCore/platform/graphics/FloatRectInsets.cpp:33
> +TextStream& operator<<(TextStream& ts, const FloatRectInsets &insets)

"FloatRectInsets &" => "FloatRectInsets& "

> Source/WebCore/platform/graphics/FloatRectInsets.h:58
> +

Should this know how to convert to UIEdgeInsets via operator UIEdgeInsets()? (like how FloatRect can convert to a CGRect).

[Tim Horton]

Created attachment 307306 [details]
Patch

[Build Bot]

Attachment 307306 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 307318 [details]
Patch

[Build Bot]

Attachment 307318 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 307327 [details]
Patch

[Build Bot]

Attachment 307327 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 307327 [details]
Patch

Clearing flags on attachment: 307327

Committed r215446: <http://trac.webkit.org/changeset/215446>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

This change broke the iOS debug build:
https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Debug%20%28Build%29/builds/843

[Sam Weinig]

Is this for Mac and iOS, or just iOS?

[Tim Horton]

(In reply to Sam Weinig from comment #16)
> Is this for Mac and iOS, or just iOS?

Just iOS. Mac sends even less! Only top. But we should use the same mechanism (the WebCore parts are not iOS specific) in the future.

[Sam Weinig]

(In reply to Tim Horton from comment #17)
> (In reply to Sam Weinig from comment #16)
> > Is this for Mac and iOS, or just iOS?
> 
> Just iOS. Mac sends even less! Only top. But we should use the same
> mechanism (the WebCore parts are not iOS specific) in the future.

Yay-boo.

[Sam Weinig]

Also, was this really not testable? You even updated TextStream to support it :|

[Tim Horton]

Settle down and wait for the next patch :)

[Sam Weinig]

(In reply to Tim Horton from comment #20)
> Settle down and wait for the next patch :)

Oh, so now I have to trust that "tests are coming"? I've been spun that tale before young man.