| Summary: | Plumb all four obscured insets to WebCore, instead of just top/left | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | andersca, bdakin, buildbot, cdumez, commit-queue, dbates, japhet, mitz, ryanhaddad, sam, simon.fraser, wenson_hsieh | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Tim Horton
2017-04-17 13:52:25 PDT
Created attachment 307293 [details]
Patch
Maybe this should use FloatBoxExtent? 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>? 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.
lol, I beat you by 10 seconds. 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). Created attachment 307306 [details]
Patch
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.
Created attachment 307318 [details]
Patch
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.
Created attachment 307327 [details]
Patch
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.
Comment on attachment 307327 [details] Patch Clearing flags on attachment: 307327 Committed r215446: <http://trac.webkit.org/changeset/215446> All reviewed patches have been landed. Closing bug. This change broke the iOS debug build: https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Debug%20%28Build%29/builds/843 Is this for Mac and iOS, or just iOS? (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. (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. Also, was this really not testable? You even updated TextStream to support it :| Settle down and wait for the next patch :) (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. |