Logic error in WebCore/Page/SpatialNavigation.cpp::areRectsPartiallyAligned

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryan Sleevi]

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));
}

[Antonio Gomes]

Nice tool! I can fix it, it you are not going to ...

[Ryan Sleevi]

Yes, please do. Sorry, I'm not familiar enough with this code to see the obvious fix.

[Antonio Gomes]

Created attachment 95122 [details]
patch

[Andreas Kling]

Landed in <http://trac.webkit.org/changeset/87616>.