61606 2011-05-26 22:31:42 -0700 Logic error in WebCore/Page/SpatialNavigation.cpp::areRectsPartiallyAligned 2011-05-29 10:32:51 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 50666 1 rsleevi webkit-unassigned hyatt kling tonikitoo yael oldest_to_newest 411164 0 rsleevi 2011-05-26 22:31:42 -0700 This was reported downstream in Chromium by a user running a static analyzer against the Chromium sources. The original bug report is http://code.google.com/p/chromium/issues/detail?id=83873#c2 , with the specific bug split into http://code.google.com/p/chromium/issues/detail?id=84140 The original report is: V501 There are identical sub-expressions '(bStart >= aStart && bStart <= aEnd)' to the left and to the right of the '||' operator. webcore_remaining spatialnavigation.cpp 236 // This method checks if |start| and |dest| have a partial intersection, either // horizontally or vertically. // * a = Current focused node's rect. // * b = Focus candidate node's rect. static bool areRectsPartiallyAligned(FocusDirection direction, const IntRect& a, const IntRect& b) { int aStart = start(direction, a); int bStart = start(direction, b); int bMiddle = middle(direction, b); int aEnd = end(direction, a); int bEnd = end(direction, b); // Picture of the partially aligned logic: // // Horizontal Vertical // ******************************** // * _ * _ _ _ * // * |_| * |_|_|_| * // * |_|.... _ * . . * // * |_| |_| * . . * // * |_|....|_| * ._._ _ * // * |_| * |_|_|_| * // * |_| * * // * * * // ******************************** // // ... and variants of the above cases. return ((bStart >= aStart && bStart <= aEnd) || (bStart >= aStart && bStart <= aEnd) || (bEnd >= aStart && bEnd <= aEnd) || (bMiddle >= aStart && bMiddle <= aEnd) || (bEnd >= aStart && bEnd <= aEnd)); } 411176 1 tonikitoo 2011-05-26 22:54:42 -0700 Nice tool! I can fix it, it you are not going to ... 411178 2 rsleevi 2011-05-26 22:55:52 -0700 Yes, please do. Sorry, I'm not familiar enough with this code to see the obvious fix. 411202 3 95122 tonikitoo 2011-05-26 23:30:16 -0700 Created attachment 95122 patch 411980 4 kling 2011-05-29 10:32:51 -0700 Landed in <http://trac.webkit.org/changeset/87616>. 95122 2011-05-26 23:30:16 -0700 2011-05-27 03:47:28 -0700 patch 0001-2011-05-26-Antonio-Gomes-agomes-rim.com.patch text/plain 2154 tonikitoo RnJvbSAzMzNiOGQwMjAzYTU3MmEyNjNkMmI3NTU5MzUzNGU0MzU0MzZmNTAyIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbnRvbmlvIEdvbWVzIDxhZ29tZXNAcmltLmNvbT4KRGF0ZTog RnJpLCAyNyBNYXkgMjAxMSAwMjoyNzo0OSAtMDQwMApTdWJqZWN0OiBbUEFUQ0hdIDIwMTEtMDUt MjYgIEFudG9uaW8gR29tZXMgIDxhZ29tZXNAcmltLmNvbT4KCiAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCgogICAgICAgIExvZ2ljIGVycm9yIGluIFdlYkNvcmUvUGFnZS9TcGF0 aWFsTmF2aWdhdGlvbi5jcHA6OmFyZVJlY3RzUGFydGlhbCAgICAgIGx5QWxpZ25lZAogICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MTYwNgoKICAgICAgICBS ZW1vdmVkIGR1cGxpY2F0ZWQgc3RhdGVtZW50IHdpdGhpbiBpZiBjb25kaXRpb24uCgogICAgICAg IE5vIG5ldyB0ZXN0cy4gT2J2aW91cyBjbGVhbiB1cCBwYXRjaC4KCiAgICAgICAgKiBwYWdlL1Nw YXRpYWxOYXZpZ2F0aW9uLmNwcDoKICAgICAgICAoV2ViQ29yZTo6YXJlUmVjdHNQYXJ0aWFsbHlB bGlnbmVkKToKCiogcGFnZS9TcGF0aWFsTmF2aWdhdGlvbi5jcHA6CihXZWJDb3JlOjphcmVSZWN0 c1BhcnRpYWxseUFsaWduZWQpOgotLS0KIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyAgICAgICAg ICAgICAgICAgIHwgICAxNCArKysrKysrKysrKysrKwogU291cmNlL1dlYkNvcmUvcGFnZS9TcGF0 aWFsTmF2aWdhdGlvbi5jcHAgfCAgICAxIC0KIDIgZmlsZXMgY2hhbmdlZCwgMTQgaW5zZXJ0aW9u cygrKSwgMSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VM b2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggY2U1MjA1ZS4uNjM0ZTlmMyAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDExLTA1LTI2ICBBbnRvbmlvIEdvbWVzICA8YWdv bWVzQHJpbS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgTG9naWMgZXJyb3IgaW4gV2ViQ29yZS9QYWdlL1NwYXRpYWxOYXZpZ2F0aW9uLmNwcDo6 YXJlUmVjdHNQYXJ0aWFsbHlBbGlnbmVkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD02MTYwNgorCisgICAgICAgIFJlbW92ZWQgZHVwbGljYXRlZCBzdGF0 ZW1lbnQgd2l0aGluIGlmIGNvbmRpdGlvbi4KKworICAgICAgICBObyBuZXcgdGVzdHMuIE9idmlv dXMgY2xlYW4gdXAgcGF0Y2guCisKKyAgICAgICAgKiBwYWdlL1NwYXRpYWxOYXZpZ2F0aW9uLmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OmFyZVJlY3RzUGFydGlhbGx5QWxpZ25lZCk6CisKIDIwMTEt MDUtMjYgIE1paGFpIFBhcnBhcml0YSAgPG1paGFpcEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg UmV2aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2Uv U3BhdGlhbE5hdmlnYXRpb24uY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9TcGF0aWFsTmF2aWdh dGlvbi5jcHAKaW5kZXggMjZiMjRhYS4uZGQ1NjUyOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvcGFnZS9TcGF0aWFsTmF2aWdhdGlvbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9T cGF0aWFsTmF2aWdhdGlvbi5jcHAKQEAgLTIzNCw3ICsyMzQsNiBAQCBzdGF0aWMgYm9vbCBhcmVS ZWN0c1BhcnRpYWxseUFsaWduZWQoRm9jdXNEaXJlY3Rpb24gZGlyZWN0aW9uLCBjb25zdCBJbnRS ZWN0JiBhLAogICAgIC8vCiAgICAgLy8gLi4uIGFuZCB2YXJpYW50cyBvZiB0aGUgYWJvdmUgY2Fz ZXMuCiAgICAgcmV0dXJuICgoYlN0YXJ0ID49IGFTdGFydCAmJiBiU3RhcnQgPD0gYUVuZCkKLSAg ICAgICAgICAgIHx8IChiU3RhcnQgPj0gYVN0YXJ0ICYmIGJTdGFydCA8PSBhRW5kKQogICAgICAg ICAgICAgfHwgKGJFbmQgPj0gYVN0YXJ0ICYmIGJFbmQgPD0gYUVuZCkKICAgICAgICAgICAgIHx8 IChiTWlkZGxlID49IGFTdGFydCAmJiBiTWlkZGxlIDw9IGFFbmQpCiAgICAgICAgICAgICB8fCAo YkVuZCA+PSBhU3RhcnQgJiYgYkVuZCA8PSBhRW5kKSk7Ci0tIAoxLjcuMQoK